[hibernate-dev] [SEARCH] Code style - imports

Tue Sep 12 05:51:50 EDT 2017

On 12 September 2017 at 10:49, Yoann Rodiere <yoann at hibernate.org> wrote:
> Well, if you don't mind me sending PRs with those imports in a different
> order next time I change the file (because I don't spend time on organizing
> imports manually, I just use CTRL+SHIFT+O), then yes, we can leave that
> undefined.

That works for me too. Just please avoid compulsive CTRL+SHIFT+O'ing
around when there's no significant change ;)

>
> Yoann Rodière
> Hibernate NoORM Team
> yoann at hibernate.org
>
> On 12 September 2017 at 11:45, Sanne Grinovero <sanne at hibernate.org> wrote:
>>
>> On 12 September 2017 at 10:42, Yoann Rodiere <yoann at hibernate.org> wrote:
>> >> Fundamentally no IDE will ever produce exactly the same code so you
>> >> need to learn control your primal lust for perfection and remember
>> >> it's not relevant. Have some decency ;)
>> >
>> >
>> > Sorry if that seemed "indecent", but I did mention that I could live
>> > with
>> > the inconsistencies, and that what matters is their effect on our git
>> > workflow :)
>> >
>> >> If you want to volunteer making the styles a bit more similar, I
>> >> typically prefer static imports last.
>> >
>> >
>> > Sure... thing is you can't do that in Eclipse, as I mentioned. I'll try
>> > to
>> > align the rest of the rules, though.
>> >
>> >> Sergey sent a related PR but I'm not sure what to think of it as it
>> >> changes several things:
>> >>  - https://github.com/hibernate/hibernate-ide-codestyles/pull/3
>> >
>> >
>> > Yes, it does more than what it advertises, and even the original idea
>> > (putting static imports to the top) is not a good idea: ORM already has
>> > static imports at the bottom in almost every file, so this change would
>> > lead
>> > to many more changes than necessary in commits for the ORM project for
>> > some
>> > time.
>> >
>> >> I would say that it would be nice to have Eclipse and Idea rules as
>> >> similar as possible. The issue with that is that, for a while, the ORM
>> >> people will have a lot of import changes, considering we can't make the
>> >> Eclipse rules Idea like but we can do the opposite.
>> >
>> >
>> > I doubt they'll ever agree with this solution. And I can't blame them.
>> >
>> >> If we don't want to bother the ORM team with that, we should split our
>> >> rules between ORM and NoORM but it won't be very underestandable for
>> >> the
>> >> contributors (but it's not as if we had a lot of them and very few of
>> >> them
>> >> find our code styles without us mentioning them).
>> >
>> >
>> > Seems to be the least disruptive solution. It's not ideal, but it should
>> > improve the situation slightly and it wouldn't lead to more changes than
>> > necessary in the future git commits.
>> > @Sanne this means the imports will stay at the top, though. Ok with
>> > that?
>>
>> Yes I'm "ok" with that. But wouldn't it be even better to leave this
>> level of detail undefined?
>>
>> At least it would allow me to make new code having them on the bottom
>> as I like them more, and you'd save some itme :)
>>
>> >
>> > I'll try to see if I can add non-disruptive checkstyle rules too, if I
>> > have
>> > some time.
>> >
>> > Yoann Rodière
>> > Hibernate NoORM Team
>> > yoann at hibernate.org
>> >
>> > On 12 September 2017 at 11:31, Guillaume Smet <guillaume.smet at gmail.com>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> Yes, I had a hard time reverting the import changes in my recent
>> >> patches
>> >> for ORM. I was changing only a few lines per file and had a lot of
>> >> import
>> >> noise.
>> >>
>> >> I would say that it would be nice to have Eclipse and Idea rules as
>> >> similar as possible. The issue with that is that, for a while, the ORM
>> >> people will have a lot of import changes, considering we can't make the
>> >> Eclipse rules Idea like but we can do the opposite.
>> >>
>> >> If we don't want to bother the ORM team with that, we should split our
>> >> rules between ORM and NoORM but it won't be very underestandable for
>> >> the
>> >> contributors (but it's not as if we had a lot of them and very few of
>> >> them
>> >> find our code styles without us mentioning them).
>> >>
>> >> --
>> >> Guillaume
>> >>
>> >>
>> >> On Tue, Sep 12, 2017 at 11:13 AM, Sanne Grinovero <sanne at hibernate.org>
>> >> wrote:
>> >>>
>> >>> Fundamentally no IDE will ever produce exactly the same code so you
>> >>> need to learn control your primal lust for perfection and remember
>> >>> it's not relevant. Have some decency ;)
>> >>>
>> >>> What does actually matter is that we don't keep chaging the order as
>> >>> it introduces conflicts on backporting, it's very annoying and time
>> >>> consuming in the long run.
>> >>>
>> >>> My solution is simple: I don't generally care, BUT commits containing
>> >>> code reformats just for the sake of making one's favourite tool happy
>> >>> need be rejected, especially to stop the bad habit from developing
>> >>> further, or anyone trying to impose its own favourite tool of the day
>> >>> as it was the only blessed way.
>> >>> It's fine if a couple of import reorg "slip in" as part as meaningful
>> >>> changes.
>> >>>
>> >>> The real problem is that applying a patch shouldn't conflict on import
>> >>> statements; ideally there should be a conflict resolution strategy
>> >>> able to understand it better (e.g. teach Java to git tools..)
>> >>>
>> >>> If you want to volunteer making the styles a bit more similar, I
>> >>> typically prefer static imports last.
>> >>>
>> >>> -1 for CheckStyle rules unless you can figure out a way to exclude
>> >>> existing code from failing: we're not going to reformat half the code
>> >>> base to satisfy aesthetics.
>> >>>
>> >>> Perhaps we could have separate sets of rules, some stricter to be
>> >>> applied on new modules; their impact would be limited but even having
>> >>> one module with strict rules would encourage people to reconfigure
>> >>> their IDE in "the right way". This would ensure all new code will be
>> >>> consistent with the rules.
>> >>>
>> >>> Sergey sent a related PR but I'm not sure what to think of it as it
>> >>> changes several things:
>> >>>  - https://github.com/hibernate/hibernate-ide-codestyles/pull/3
>> >>>
>> >>> Thanks,
>> >>> Sanne
>> >>>
>> >>>
>> >>>
>> >>> On 12 September 2017 at 08:42, Yoann Rodiere <yoann at hibernate.org>
>> >>> wrote:
>> >>> > Hello all,
>> >>> >
>> >>> > I just noticed that our IDEA codestyle configuration files have
>> >>> > specific
>> >>> > rules about how to organize imports, whereas the Eclipse ones don't.
>> >>> > The most noticeable difference: organizing imports in Eclipse puts
>> >>> > static
>> >>> > imports at the top, while in IDEA they end up at the bottom. But
>> >>> > there
>> >>> > are
>> >>> > other differences.
>> >>> > The Hibernate ORM codebase doesn't seem affected by these
>> >>> > inconsistencies
>> >>> > (probably because most people working on it use IDEA), but in
>> >>> > Hibernate
>> >>> > Search we've already reached the point where half of our files
>> >>> > follow
>> >>> > one
>> >>> > convention, and the other half follows the other.
>> >>> >
>> >>> > What's annoying here is not the inconsistencies themselves (I could
>> >>> > live
>> >>> > with that), it's more that in the long run we'll end up with tons of
>> >>> > unnecessary changes in our commits, due to one person using IDEA to
>> >>> > edit a
>> >>> > file last edited with Eclipse, and vice-versa.
>> >>> >
>> >>> > To make the matter worse, Eclipse is not as flexible as IDEA, and we
>> >>> > cannot
>> >>> > configure Eclipse to match the IDEA style exactly. In particular, we
>> >>> > cannot
>> >>> > configure where the static imports must go: they will always go to
>> >>> > the
>> >>> > top (
>> >>> > https://bugs.eclipse.org/bugs/show_bug.cgi?id=304415).
>> >>> >
>> >>> > I can see two ways out, each with its own drawbacks:
>> >>> >
>> >>> >    - Switch to the "Eclipse" style, and create a Search-specific
>> >>> > IDEA
>> >>> > code
>> >>> >    style configuration, with static imports at the top. We won't be
>> >>> > consistent
>> >>> >    with Hibernate ORM, though.
>> >>> >    - Switch to the "IDEA" style, and create a Search-specific
>> >>> > Eclipse
>> >>> > code
>> >>> >    style configuration, as close as the IDEA style as possible. As I
>> >>> > said
>> >>> >    above, automatic import organizing in Eclipse will never follow
>> >>> > this
>> >>> > style
>> >>> >    exactly, so it's likely to make organizing imports in Eclipse
>> >>> > quite
>> >>> >    painful. We could add checkstyle rules to make sure we strictly
>> >>> > follow the
>> >>> >    style even in Eclipse.
>> >>> >
>> >>> > WDYT?
>> >>> >
>> >>> >
>> >>> > Yoann Rodière
>> >>> > Hibernate NoORM Team
>> >>> > yoann at hibernate.org
>> >>> > _______________________________________________
>> >>> > hibernate-dev mailing list
>> >>> > hibernate-dev at lists.jboss.org
>> >>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >>>
>> >>> _______________________________________________
>> >>> hibernate-dev mailing list
>> >>> hibernate-dev at lists.jboss.org
>> >>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >>
>> >>
>> >
>
>