Removed a lot of compiler warnings #5487
Conversation
Inserted diamond in constructions like this: ArrayList<Strings> = new ArrayList(); No any interfaces was changed
|
please don't ever merge PRs which don't have tests run on them! This PR broke master. There is no need to hurry. This is a project wide cleanup which requires all tests to be run which is activated with the |
As someone who has previously worked on things in this area, I spent the time to look at each file that was changed. So I'll accept the responsibility for what happened. It appeared to me that the tests were being "skipped" by design. Previously when I've looked at the test runs, the system always showed as "failed". So it was clearly my mistake. My apologies to everyone! I'll be more diligent next time! |
|
the changes itself were fine, the problem was that some were in test data. Just don't be so quick with the merge button next time. It is perfectly fine to let a PR sit a few days - increases the probability that someone notices things like this, e.g missing label to run all tests. |
Agreed. I looked at each change. But, making a change in test code is definitely a danger. Another good reason to keep these type of PRs smaller in size. |
partial revert of #5487 to fix master.
Fully agree. These huge PRs that change many files are prone to give problems. We should balance the benefits and the risks of these PRs. And don't worry, @BradWalker ! This is something that could happen to anyone! |
@vieiro @BradWalker yes! This happens sometimes, so please don't worry about it. Its just that if you would have waited with the merge 1-2 days after review, I am certain someone would have noticed. why? because I for example try to look at https://github.com/apache/netbeans/actions on a daily basis to check if there is something to play whack-a-mole with, that is also how I noticed this in the first place. When someone syncs a new or old PR I would notice this too (since it appears on the all-knowing list) and probably check if its properly labeled. IMO the main lesson learned here should be to not rush PRs in, even if they seem trivial and were carefully reviewed. This could have happened with a small PR too which only covers one module or less - it likely would have required some of the [ci] labels too. Java PRs need Java, PHP needs PHP otherwise the correct tests won't run etc etc. |
|
also regarding large cleanup PRs in general: We can't split everything into PRs otherwise every project wide cleanup would mean 18 PRs, one for each cluster - lets not do that. |
Inserted diamond in constructions like this: ArrayList<Strings> = new ArrayList(); No any interfaces was changed
Inserted diamond in constructions like this:
ArrayList = new ArrayList();
No any interfaces was changed