Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Extend Browser API to check for custom text location" #1533

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Revert "Extend Browser API to check for custom text location" #1533

merged 1 commit into from
Oct 14, 2024

Conversation

amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented Oct 11, 2024 •
edited
Loading

This reverts commit 51b8d8f. The API isLocationForCustomText is not needed for Browsers.

contributes to #213

@amartya4256
Copy link
Contributor Author

@HeikoKlare Do we need to do another version bump?

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 11, 2024 •
edited
Loading

Test Results

   486 files  ± 0     486 suites  ±0   7m 28s ⏱️ -49s
 4 154 tests  -  5   4 146 ✅  -  5   8 💤 ±0  0 ❌ ±0 
16 370 runs   - 20  16 278 ✅  - 20  92 💤 ±0  0 ❌ ±0 

Results for commit 7fbee5e. ± Comparison against base commit 8ce60f9.

This pull request removes 5 tests. 
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_isLocationForCustomText
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_isLocationForCustomText_isFirstSetTextURLStillCustomTextUrlAfterSetUrl
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_isLocationForCustomText_isSetUrlNotCustomTextUrl
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_isLocationForCustomText_isSetUrlNotCustomTextUrlAfterSetText
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_isLocationForCustomText_setUrlAfterDisposedThrowsSwtException

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare force-pushed the create_api_forbrowser_location_check branch 3 times, most recently from 639fa7b to acb2b9b Compare October 14, 2024 14:22
@sratz
Copy link
Member

sratz commented Oct 14, 2024

This reverts public API introduced in the last release. It is not very likely anyone has adopted that yet but I don't think we can simply revert this now.

In fact, the API tools should suggest a major version bump for this change - not sure why this is not the case (or I am just missing it?)

@HeikoKlare
Copy link
Contributor

It has been introduced after the last release for M1, so it should be safe to revert now, shouldn't it?

@sratz
Copy link
Member

sratz commented Oct 14, 2024

It has been introduced after the last release for M1, so it should be safe to revert now, shouldn't it?

Ah, you're right. Yes, then it's perfectly fine to revert now without a version bump.

And yes, we should do so as early as possible to get this into M2.

@HeikoKlare HeikoKlare force-pushed the create_api_forbrowser_location_check branch from acb2b9b to f8a7c72 Compare October 14, 2024 15:11
@HeikoKlare
Copy link
Contributor

3 new issues of type The method onFailure() from the type Screenshots is deprecated introduced by method deprecation in eclipse-platform/eclipse.platform.releng.aggregator#2446, so unrelated to this PR. Only occurs because no SWT master build as new reference happened since then.

@amartya4256 @HeikoKlare
Revert "Extend Browser API to check for custom text location"
7fbee5e 
This reverts commit 51b8d8f. The API
isLocationForCustomText is not needed for Browsers.

contributes to eclipse-platform#213
@HeikoKlare HeikoKlare force-pushed the create_api_forbrowser_location_check branch from f8a7c72 to 7fbee5e Compare October 14, 2024 17:41
@HeikoKlare
Copy link
Contributor

Recent master build now shows the same number of issues than in this PR, but Jenkins still compares builds for this PR against previous master build:
{38C0DE4E-E7C1-431A-A7D0-1DCACB79ABFC}

@HeikoKlare HeikoKlare merged commit 8fe0fb0 into eclipse-platform:master Oct 14, 2024
11 of 14 checks passed
@HeikoKlare HeikoKlare mentioned this pull request Oct 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sratz sratz sratz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amartya4256 @sratz @HeikoKlare