Skip to main content

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

Hi,

I also strongly feel these "cleanups" are not worth it.
I'd love to see a summary of all the bugs introduced by this (maybe a dedicated keyword in bugzilla, if the practice continues?). And they are still being discovered at RC stage, who knows how many will slip through? There's also some extra confusion related to bugs that look like they're probably caused by a "cleanup", but actually are not.

What's the benefit? Less warnings reported by static analysis tools, slightly better readability of the code (most of which may not even be looked at in years).

And yes, one may always argue that in a perfect world this kind of cleaning would not cause any problems, so it's actually revealing preexisting problems. But how relevant are they? A lot of this code has been used for years by plenty of people and nobody cared that some best practices were not applied.

Regards,
Mateusz

wt., 26 maj 2020 o 20:55 Stephan Herrmann <stephan.herrmann@xxxxxxxxx> napisał(a):
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

Back to the top