Thanks Matt and Gary! I've just made a PR at https://github.com/apache/commons-lang/pull/276.
Jonathan On 10 July 2017 at 03:47, Gary Gregory <garydgreg...@gmail.com> wrote: > On Jul 9, 2017 18:49, "Matt Sicker" <boa...@gmail.com> wrote: > > I personally don't see the point of making a jira issue for it. > > > Agreed. > > Gary > > > On 9 July 2017 at 20:19, Jonathan Bluett-Duncan <jbluettdun...@gmail.com> > wrote: > > > Okay, thanks for the clarification Gary. > > > > Does this mean, by extension, that there's no need to create a new JIRA > > issue? In other words, would just a new GitHub PR be fine (at least for > > now)? > > > > Jonathan > > > > On 10 July 2017 at 01:32, Gary Gregory <garydgreg...@gmail.com> wrote: > > > > > I would think an ICLA is not needed if the only thing we are talking > > about > > > are clean-up style refactoring. > > > > > > Gary > > > > > > On Sun, Jul 9, 2017 at 2:27 PM, Jonathan Bluett-Duncan < > > > jbluettdun...@gmail.com> wrote: > > > > > > > I've re-read the contribution guidelines > > > > <https://github.com/apache/commons-lang/blob/master/CONTRIBUTING.md > >, > > > but > > > > it's not clear to me if my changes are non-trivial enough to warrant > a > > > new > > > > JIRA issue. > > > > > > > > Can someone advise me on this? > > > > > > > > Jonathan > > > > > > > > On 6 July 2017 at 00:51, Jonathan Bluett-Duncan < > > jbluettdun...@gmail.com > > > > > > > > wrote: > > > > > > > > > Okay, I don't intend to apply any `@SuppressWarnings` during my > > > > > refactoring efforts - if IntelliJ still reports warnings after my > > > > efforts, > > > > > then I can live with that. :) > > > > > > > > > > Jonathan > > > > > > > > > > On 6 July 2017 at 00:31, Matt Sicker <boa...@gmail.com> wrote: > > > > > > > > > >> The only thing I can think of that conflicts sometimes with IDEA > > > versus > > > > >> Eclipse are custom @SupressWarnings strings causing warnings in > > > Eclipse. > > > > >> The default IntelliJ warnings tend to be simple things that > wouldn't > > > > cause > > > > >> an issue, however, based in my experience. It's only some of the > > > > advanced > > > > >> inspections that can get strange. > > > > >> > > > > >> On 5 July 2017 at 17:24, Jonathan Bluett-Duncan < > > > > jbluettdun...@gmail.com> > > > > >> wrote: > > > > >> > > > > >> > Great! Sounds like there's general agreement on me pursuing at > > least > > > > >> some > > > > >> > of these refactorings, right? > > > > >> > > > > > >> > On the subject of code style guidelines, AFAIK [lang] uses > > > Checkstyle > > > > to > > > > >> > check style adherence? So I assume that if the corresponding > Maven > > > > goal > > > > >> > passes after a refactoring, then it's okay to submit it as a > > GitHub > > > > PR. > > > > >> > > > > > >> > I don't expect changes suggested by IDEA to cause > warnings/errors > > in > > > > >> > Eclipse or for the Eclipse Java compiler either. Is there an > > > automated > > > > >> way > > > > >> > to explicitly check for things like this through Apache's > > > > >> infrastructure? > > > > >> > Or would I need to manually download Eclipse and check on my > > > machine? > > > > >> > > > > > >> > I've already created an Apache JIRA account and signed the CLA, > > but > > > > now > > > > >> > it's not clear to me what to do next. Can someone kindly advise > me > > > on > > > > >> the > > > > >> > next step? > > > > >> > > > > > >> > Jonathan > > > > >> > > > > > >> > On 5 July 2017 at 20:33, Gary Gregory <garydgreg...@gmail.com> > > > wrote: > > > > >> > > > > > >> > > Keep in mind that not all of us use IDEA. For example, I am on > > > > >> Eclipse. I > > > > >> > > do not think this should be an issue for any of these changes > > > > thougg. > > > > >> I > > > > >> > do > > > > >> > > not expect that changes from IDEA warnings would cause the > > Eclipse > > > > >> Java > > > > >> > > compiler to issue warnings, and vice-versa. > > > > >> > > > > > > >> > > Gary > > > > >> > > > > > > >> > > On Jul 5, 2017 12:23, "Allon Mureinik" <murei...@gmail.com> > > > wrote: > > > > >> > > > > > > >> > > > I've submitted several such cleanups over the past couple of > > > > month, > > > > >> and > > > > >> > > for > > > > >> > > > the most part, they've been well received. > > > > >> > > > > > > > >> > > > I think the key here is to improve the codebase when > possible > > > but > > > > to > > > > >> > > leave > > > > >> > > > room to deviate from IntelliJ's norms when there's a good > > reason > > > > to. > > > > >> > > > Perhaps annotating such places with @SuppressWarning would > be > > > the > > > > >> best > > > > >> > > > approach, to signal to future developers that the warning > was > > > > >> > considered, > > > > >> > > > and we explicitly decided to suppress it (possibly with a > > > comment > > > > in > > > > >> > the > > > > >> > > > code explaining why). > > > > >> > > > > > > > >> > > > > > > > >> > > > On Wed, Jul 5, 2017 at 6:42 PM, Matt Sicker < > boa...@gmail.com > > > > > > > >> wrote: > > > > >> > > > > > > > >> > > > > I like the idea myself at least, though I'm not sure if it > > > > >> conflicts > > > > >> > > with > > > > >> > > > > any coding styleguides established. > > > > >> > > > > > > > > >> > > > > On 4 July 2017 at 18:18, Jonathan Bluett-Duncan < > > > > >> > > jbluettdun...@gmail.com > > > > >> > > > > > > > > >> > > > > wrote: > > > > >> > > > > > > > > >> > > > > > Hi all, > > > > >> > > > > > > > > > >> > > > > > I'm interested in going through commons-lang with > IntelliJ > > > > IDEA > > > > >> and > > > > >> > > > > > applying small refactorings to make the code base easier > > to > > > > read > > > > >> > > and/or > > > > >> > > > > > more performant (and also make IntelliJ IDEA itself > report > > > > less > > > > >> > > > > warnings). > > > > >> > > > > > > > > > >> > > > > > Is this something that the [lang] team would find > useful? > > > > >> > > > > > > > > > >> > > > > > Examples of refactorings that I could apply include: > > > > >> > > > > > > > > > >> > > > > > - Replace manual array-to-collection copy operations > > with > > > > >> > > > > > `Collections.addAll()`. > > > > >> > > > > > - Replace simple `String{Buffer,Builder}` usages with > > > > direct > > > > >> > > > `String` > > > > >> > > > > > concatenations or `String.format()`. > > > > >> > > > > > - Simplifying boolean expressions like `obj > instanceof > > > > >> CharRange > > > > >> > > == > > > > >> > > > > > false` to `!(obj instanceof CharRange)`. > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > Cheers, > > > > >> > > > > > Jonathan > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > -- > > > > >> > > > > Matt Sicker <boa...@gmail.com> > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > >> > > > > >> -- > > > > >> Matt Sicker <boa...@gmail.com> > > > > >> > > > > > > > > > > > > > > > > > > > > > > > -- > Matt Sicker <boa...@gmail.com> >
