Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jdt-dev] "clean up" again

I see them as disjointed activity. I think fixing a bug shouldn't cover clean-up/re-factoring of the classes, this makes reviewing the bugfix much harder. This is of course up to the committer, if the file is small maybe a full cleanup is possible without making reviewing too difficult.

Regards,

Kenneth

Den 2020-05-27 kl. 16:40, skrev stephan.herrmann@xxxxxxxxx:

To really get the full picture, I would very much like to hear from our new contributors, how they see the connection between clean up changes and functional changes / bug fixes.

Is there any connection or are these disjoint activities?

If connected, how exactly?

thanks,
Stephan

Am 2020-05-27 16:21, schrieb stephan.herrmann@xxxxxxxxx:

Still one more suggestion / request:

Let's please discuss this JDT issue as a JDT issue only.

Platform is different. Hence also the p.o.v. of the Eclipse PMC is different from the day-to-day work in JDT.

Let's find out what's best for JDT.

thanks,
Stephan

Am 2020-05-27 13:10, schrieb stephan.herrmann@xxxxxxxxx:

In my post I mainly wanted to raise awareness that JDT code (even if x-internal) is potentially consumed outside JDT, and that even seemingly trivial changes can (and do) cause havoc downstream.

Now that the discussion has been broadened to the general issue of clean ups, I would like to list three kinds of clean-ups that I do consider useful:

1. Refactorings that help fixing a bug. This could be (a) a refactoring as part of the process of understanding some old code section, or (b) a refactoring that prepares for the desired solution.

2. Changes that improve the ability to detect potential bugs using JDT's own analysis, like avoiding raw types, adding null annotations (careful when API is affected!).

3. Refactorings that are performed for the purpose of testing our own functionality in a dog-fooding like approach.


I suggest that (1) and (2) are encouraged on our productive code base, and that branches are created for experiments in (3). These branches can be made available for voluntary field testing but should not be merged to master.

Types (1) and (2) need a bugzilla for every change.

If (3) is performed on a branch, perhaps one umbrella bug can cover several experiments.

best,
Stephan


Am 2020-05-26 20:55, schrieb Stephan Herrmann:

Hi,

Another episode in the question whether clean up changes are worth the effort they cause.

Today the Object Teams build got broken by https://git.eclipse.org/r/#/c/155226/ (which doesn't even have a bug that I could re-open).

Object Teams has tons of tests for checking that we don't break JDT. In that context we have a subclass of org.eclipse.jdt.testplugin.JavaProjectHelper. This no longer compiles since the above change.

Granted, the package is marked x-internal, so JDT has permission to change any way we want.

OTOH note that every project that extends JDT is potentially interested in using also code from the JDT test suite. Here we speak of a fairly large number of projects.

I would not complain if the change was necessary to implement new functionality or fix a bug, that's certainly covered by x-internal. But I strongly doubt that this "clean up" has a benefit that justifies the consequences.

What problem is solved by adding private constructors? Are you doing it just because it is possible? The commit message doesn't indicate you even thought of the possibility that s.o. would subclass those classes.

It's too late for changing the code, because I need to fix this today for M3.

But please keep this in mind when doing further clean-up.

Stephan
_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev



_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev



_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev



_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev

Back to the top