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.

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

Correct bundle version after changes in MultiPageEditorPart #2476

Merged

Conversation

HeikoKlare
Copy link
Contributor

Recent API changes to MultiPageEditorPart and IWorkbenchPreferenceConstants have been introduced after 3.133 has already been released. Thus, the micro version bump made since then is insufficient. This change corrects the version but and the according since tags to 3.134.

Fixes API errors introduced by #2224 and #2461:
image

@HeikoKlare HeikoKlare changed the title Correct version of changes in MultiPageEditorPart Correct bundle version after changes in MultiPageEditorPart Nov 1, 2024
Copy link
Contributor

github-actions bot commented Nov 1, 2024

Test Results

 1 821 files  ±0   1 821 suites  ±0   1h 48m 33s ⏱️ - 1m 40s
 7 724 tests ±0   7 496 ✅ ±0  228 💤 ±0  0 ❌ ±0 
24 333 runs  ±0  23 586 ✅ ±0  747 💤 ±0  0 ❌ ±0 

Results for commit 4f6a56c. ± Comparison against base commit 1170414.

♻️ This comment has been updated with latest results.

Recent API changes to MultiPageEditorPart and
IWorkbenchPreferenceConstants have been introduced after 3.133 has
already been released. Thus, the micro version bump made since then is
insufficient. This change corrects the version but and the according
since tags to 3.134.
@HeikoKlare HeikoKlare marked this pull request as ready for review November 1, 2024 20:26
@HeikoKlare
Copy link
Contributor Author

As another confirmation that this is the right thing to do: the missing version bump was event reported in the Jenkins builds of #2224, see, e.g., https://github.com/eclipse-platform/eclipse.platform.ui/runs/32320535119

[API ERROR] File MANIFEST.MF at line 5: The minor version should be incremented in version 3.133.100, since new APIs have been added since version 3.133.0 (location: /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF)

@HannesWell
Copy link
Member

As another confirmation that this is the right thing to do: the missing version bump was event reported in the Jenkins builds of #2224, see, e.g., https://github.com/eclipse-platform/eclipse.platform.ui/runs/32320535119

This is also visble in the log of the latest master build:
https://ci.eclipse.org/platform/job/eclipse.platform.ui/job/master/747/consoleFull

[INFO] --- tycho-apitools:4.0.8:verify (verify) @ org.eclipse.ui.workbench ---
[INFO] Resolve API baseline for org.eclipse.platform:org.eclipse.ui.workbench:eclipse-plugin:3.133.100-SNAPSHOT with linux/gtk/x86_64
[INFO] API Analysis finished in 42 s.
[ERROR] [API ERROR] File MANIFEST.MF at line 5: The minor version should be incremented in version 3.133.100, since new APIs have been added since version 3.133.0 (location: /home/jenkins/agent/workspace/eclipse.platform.ui_master/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF)
[WARNING] 1 API problems can't be mapped to the compiler log!

I really wonder why this doesn't fail the build? @laeubi any idea? Since the error is detected, I would first assume the problem in the integration with Tycho, not in the API-tools itself.

@laeubi
Copy link
Contributor

laeubi commented Nov 4, 2024

Both the maven and compiler checks have shown additional issues here:

grafik

so at best reviewers would check before merge that we do not add new issues, the main build even shows one more

grafik

Other than that If I remember correctly we currently use an older version of API tools, that suffers from a bug where the failOnError is not accounted in all cases see here

@iloveeclipse
Copy link
Member

have shown additional issues here:

But why they are both "green" ? Shouldn't they have red crosses?

@iloveeclipse
Copy link
Member

Other than that If I remember correctly we currently use an older version of API tools, that suffers from a bug where the failOnError is not accounted in all cases see here

@laeubi : Is there a new version available? If yes, could you please update aggregator to use it?

@laeubi
Copy link
Contributor

laeubi commented Nov 4, 2024

But why they are both "green" ? Shouldn't they have red crosses?

Because platform.ui has chosen to make them green:

recordIssues(publishAllIssues:false, ignoreQualityGate:true,

Is there a new version available? If yes, could you please update aggregator to use it?

There was some issues with deadlocks but no one cared enough to analyses the root cause so an older API-Tools version was used in the meanwhile. I have now created a PR to configure the changed behavior now but it is not yet released:

@iloveeclipse
Copy link
Member

Because platform.ui has chosen to make them green:

eclipse.platform.ui/Jenkinsfile

@jukzi: I'm not sure it was meant to "mute" API checks: #1739

We should revert this to make API errors "visible" again.

@BeckerWdf
Copy link
Contributor

As another confirmation that this is the right thing to do: the missing version bump was event reported in the Jenkins builds of #2224, see, e.g., https://github.com/eclipse-platform/eclipse.platform.ui/runs/32320535119

[API ERROR] File MANIFEST.MF at line 5: The minor version should be incremented in version 3.133.100, since new APIs have been added since version 3.133.0 (location: /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF)

Thanks at @HeikoKlare for taking care. I simply overlooked this.

@iloveeclipse iloveeclipse merged commit 56115fc into eclipse-platform:master Nov 4, 2024
17 checks passed
@HeikoKlare HeikoKlare deleted the worbench-version-bump branch November 4, 2024 07:13
@jukzi
Copy link
Contributor

jukzi commented Nov 4, 2024

We should revert this to make API errors "visible" again.

https://github.com/eclipse-platform/eclipse.platform.ui/pull/1739/files did not mute warnings. see #1739 (comment), also proofen by the other PRs failing by the warnings.
In the particular case of https://github.com/eclipse-platform/eclipse.platform.ui/runs/32320534400 the "problem" is that same number of warnings have been identified fixed as solved:
image

@iloveeclipse
Copy link
Member

We should revert this to make API errors "visible" again.

https://github.com/eclipse-platform/eclipse.platform.ui/pull/1739/files did not mute warnings. see #1739 (comment), also proofen by the other PRs failing by the warnings. In the particular case of https://github.com/eclipse-platform/eclipse.platform.ui/runs/32320534400 the "problem" is that same number of warnings have been identified fixed as solved: image

Please comment on #2486.

I honestly don't know what exactly causes github actions be green despite finding new issues, but @laeubi seems to know that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants