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've already expressed my concerns before, but I was told that this is the best way to attract new contributors, and that one should not complain about contributions but be happy that someone touches our bad dirty code.
 
In *my* environment all people that know about this mass changes are only shaking heads about this nonsense. I can't count how many times I've reverted or fixed bugs coming from that, it is a constant pain and constant time killer for me. Backporting or reverting commits without merge issues is almost impossible, because we have numerous commits with gazillion of touched files across the code base. Git history is almost useless now. For one real file change we have 5 various "optimizations" just moving bits around.
 
I personally would understand and accept cleanups only before or after a planned bug fix in the related code.
Cleanups that do *functional* changes (not just white space) shouldn't be done "just because we can".
 
Unfortunately this seem to be a mass sport today and Eclipse platform code is used as a playground for experiments - everyone is welcome to apply "refactoring of the day" on Eclipse code base, "because we can" and because that "attracts new contributors". I doubt that buggy code is attracting and that platform master branch is a good playground for refactorings, but probably I'm alone with this opinion.
 
One explanation (I guess) why some are doing that is that people get job "marketing" for free by pointing on the project stats / git logs and saying "look, I'm Eclipse committer".
This would be OK, if that patches would not harm, and if the people would monitor incoming bugs and fix regressions, but I don't see this happening.
 
Kind regards,
Andrey Loskutov

Спасение утопающих - дело рук самих утопающих

https://www.eclipse.org/user/aloskutov
 
 
Gesendet: Dienstag, 26. Mai 2020 um 23:53 Uhr
Von: "Mateusz Matela" <mateusz.matela@xxxxxxxxx>
An: "Eclipse JDT general developers list." <jdt-dev@xxxxxxxxxxx>
Betreff: 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
_______________________________________________ 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