Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jdt-core-dev] Minimal-change policy?

The purpose of such refactoring is not just for beauty and consistency. Making our code compatible with our tools means that everyone doesn't have to waste time fighting against our automated tools whenever they save a file. Sure we have to pay a one-time merge cost, but it saves time on every subsequent edit of that file and should still be a net time saving. The beauty and consistency are just a bonus..


On Thu, Oct 6, 2016 at 3:55 PM Stefan Xenos <sxenos@xxxxxxxxxx> wrote:
Also during the new index work, we got a new dependency. We now require com.ibm.icu.

Sorry about that. Please file a bug for me and mention the alternative and I'll remove it.

Some files had huge changes in imports, apparently by using organize imports

It's really hard to avoid these, or even know that they occurred until review time. Organize imports and post-save actions all really like to bring these changes back and then code folding hides them from you until review-time.

Perhaps we should do a one-time organize imports on the entire JDT core code base that way it would no longer be necessary to fight against the post-save actions. I've often had these changes come back in the very change I'm writing to revert them. That will give you a single commit you can avoid or - knowing that its coming - you can do the same operation on your branch prior to the merge so that the merge goes cleanly.

SourceTypeBinding and SpaceSeparator were transformed without mentioning in the bug

I see it mentioned in bug 497419.

The line endings had to be fixed eventually and an unpleasant merge was inevitable when it happened... and delaying the fix forever to avoid the unpleasant merge isn't a solution. You could fix the line endings locally in your branch before doing the merge, but I understand that that's still a lot more work than letting the automerge do its thing.

  - Stefan

On Thu, Oct 6, 2016 at 2:03 PM Stephan Herrmann <stephan.herrmann@xxxxxxxxx> wrote:
Hi Team,

As most of you know aside from working in JDT I also maintain a fork
of JDT/Core for the Object Teams project.

Today I merged a big chunk of changes from JDT/Core into Object Teams
and found some changes worth commenting:


Some files had changes in line endings. While changes to test classes
had been discussed [1], classes SourceTypeBinding and SpaceSeparator
were transformed without mentioning in the bug. Merging just these
two files caused significant pain, eating my time that otherwise could
have been time to fix a bug in JDT.

Some files had huge changes in imports, apparently by using organize
imports, that transformed a few .* imports into pages of individual
imports and included re-sorting. I believe these happened during the
new index work. While also this can be seen as a clean-up, it causes
pain during merging.

These issues seem to be specific to Object Teams, but I think that
Groovy Eclipse is in the same boat, and given that the latter is
no longer officially supported by Pivotal, any difficulty during
merging could effectively kill that plugin, sooner or later.

So, please everybody keep in mind, what looks like nice clean-up,
can cause pain to others. If such operations are deemed necessary
for some reason, I'd appreciate if they are announced, and carried
out by a dedicated commit with no other changes, so that I can simply
skip that commit (and perform the clean-up by myself).


Also during the new index work, we got a new dependency. We now
require com.ibm.icu. Considering that this addition was motivated
by exactly one method call (for which an alternative exists),
and also considering that people are using o.e.jdt.core outside
Eclipse for various standalone tools, I think this, too, doesn't
carry its own weight. Can this be reconsidered?


best,
Stephan


[1] https://bugs.eclipse.org/497419
_______________________________________________
jdt-core-dev mailing list
jdt-core-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jdt-core-dev

Back to the top