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

Remove unnecessary casts #647

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Conversation

stephan-herrmann
Copy link
Contributor

When eclipse-jdt/eclipse.jdt.core#2471 is merged, ecj will signal more casts as unnecessary.

With this PR I suggest to remove affected casts before new warnings will show up in the build.

@akurtakov
Copy link
Member

Version bump needed:
Failed to execute goal org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.8:compare-version-with-baselines (compare-attached-artifacts-with-release) on project org.eclipse.equinox.app: Only qualifier changed for (org.eclipse.equinox.app/1.7.100.v20240620-1529). Expected to have bigger x.y.z than what is available in baseline (1.7.100.v20240321-1445) -> [Help 1]

Copy link

github-actions bot commented Jun 20, 2024

Test Results

  660 files  ±0    660 suites  ±0   1h 11m 32s ⏱️ +34s
2 195 tests ±0  2 148 ✅ ±0   47 💤 ±0  0 ❌ ±0 
6 729 runs  ±0  6 586 ✅ ±0  143 💤 ±0  0 ❌ ±0 

Results for commit 375d218. ± Comparison against base commit 029433d.

♻️ This comment has been updated with latest results.

@stephan-herrmann
Copy link
Contributor Author

testCoordinatedConfigurationOnBeforeRegisteredManagedService (org.osgi.test.cases.cm.junit.CMCoordinationTestCase) failed

Sorry, I can't find that test so I cannot test if this failure could possibly be related.

@tjwatson
Copy link
Contributor

Sorry, I can't find that test so I cannot test if this failure could possibly be related.

It is not related.

@stephan-herrmann
Copy link
Contributor Author

Sorry, I can't find that test so I cannot test if this failure could possibly be related.

It is not related.

thanks.

@merks
Copy link
Contributor

merks commented Jun 20, 2024

I think it’s best for there to be one commit not 3.

I think I can do that with squash and merge button, but I’ve not tried that before.

@merks merks merged commit 50e03e5 into eclipse-equinox:master Jun 20, 2024
26 of 27 checks passed
@merks
Copy link
Contributor

merks commented Jun 20, 2024

That worked okay.

@stephan-herrmann
Copy link
Contributor Author

I think I can do that with squash and merge button, but I’ve not tried that before.

There's a first time for everything :)

(On other occasions people wanted to have version bumps separate from code changes, but perhaps that's only relevant in projects with BETA_JAVAXY branches).

@stephan-herrmann stephan-herrmann deleted the removeCasts branch June 20, 2024 18:10
@merks
Copy link
Contributor

merks commented Jun 20, 2024

Yes sometimes people want that. I think to make revert easier. But we don’t expect to revert this and the preceding state would never build successfully. 😬

@merks
Copy link
Contributor

merks commented Jun 20, 2024

And probably two separate version bump commits never make sense. 😱

Personally I generally amend and force push until I’m done. No one has complained so far. 😜

@tjwatson
Copy link
Contributor

Yes sometimes people want that. I think to make revert easier. But we don’t expect to revert this and the preceding state would never build successfully.

Yes, that was the reason for the policy. Generally, it caused problems if we reverted and one I-Build to the next we had a version decrease. The other issue was if other changes happened after then the version decrease would become an issue also. But a straight revert in this project seems pretty rare vs. a new PR that fixes the issue on top of the existing changes.

@merks
Copy link
Contributor

merks commented Jun 20, 2024

Nothing I commit ever needs to be reverted. 👼 😜

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.

4 participants