Hello, My 2 cents about direct merge : nerver do that. What you are thinking trivial change may be not.
Regards Arnaud (more user than contributor) Le lun. 29 déc. 2025, 19:02, Michael Bien <[email protected]> a écrit : > On 12/29/25 03:08, Michael Bien wrote: > > Hi Brad, > > > > given how many tests have to run for each PR and how many javac warnings > > the code base still produces, i would recommend to group similar > > changes to reviewable changesets instead of committing them one by one - > even if trivial. > > > > (building with -Xmaxwarns 10000 generates a 36MB large build log, so the > only realistic way to > > make progress on that front is likely to fix them in batches I believe.) > > > > Regarding integration without review: its probably ok to do that for > trivial changes but > > I would still wait at least a few days to give others a chance to look > at the PR before merging > > if there is no hurry. Generally all committers can commit, review is a > tool to communicate > > to others what changes are made to the repo and improve overall quality > - sometimes > > this isn't needed. > > > > Regarding the "redundant cast" warning you mentioned. I noticed that > this particular editor hint > > is actually disabled in the project settings > > (nbbuild/misc/hints-settings.xml b/nbbuild/misc/hints-settings.xml). > > > > That is likely why this warning type made it into the code since it > wasn't visible in the editor. > > We could probably enable all default javac warnings there. > > I took a better look and we probably don't have to change anything in the > project settings. > > There are two hints with overlapping functionality: > > The RemoveUselessCast hint which intercepts the javac warning directly > (enabled based on javac setting), > and TooStrongCast which is disabled by default and can do more than basic > cast removal. > > I got confused due to the fact that RemoveUselessCast can't be run as code > inspection, while TooStrongCast can. > But the javac warning does in fact show up in the editor and is also > fixable from there (via RemoveUselessCast). > > (It might be a limitation of the hint system that ErrorRule based hints > can't be run as inspections) > > best regards, > > michael > > > > > > Dependent on how many pop up, this could be fixed on a per-cluster basis > or even project-wide. > > > > feel free to add me as reviewer if you want ;) > > > > best regards, > > > > michael > > > > On 12/29/25 02:16, Brad Walker wrote: > >> Like many folks in this group, I have commit privileges. > >> > >> As part of the code cleanup work I do, quite often other "issues" pop up > >> like this, for example. > >> > >> [nb-javac] > >> > /home/bwalker/src/netbeans/platform/core.startup/src/org/netbeans/core/startup/layers/BinaryFS.java:545: > >> warning: [cast] redundant cast to ByteBuffer > >> [nb-javac] ByteBuffer sub = > >> > (ByteBuffer)content.duplicate().order(ByteOrder.LITTLE_ENDIAN).position(offset); > >> [nb-javac] ^ > >> > >> This warning is so straightforward and simple to fix. Basically remove > the > >> cast. > >> > >> Given that resources are constrained, I wonder if it's "necessary" for > me > >> to submit a code review if all I'm doing is removing the redundant cast. > >> Rather just merge it? > >> > >> I ask this question for several reasons: 1 - I really don't want to > burden > >> the group with something as simple as this and 2 - Resources are really > >> tight and for something as mundane as this, is it really necessary. > >> > >> Really all I'm looking for here is some guidance that helps. > >> > >> -brad w. > >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > For further information about the NetBeans mailing lists, visit: > https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists > > > >
