Skip to main content

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



On Wed, May 27, 2020 at 1:30 AM Andrey Loskutov <loskutov@xxxxxx> wrote:
I've already expressed my concerns before, but I was told that this is


the best way to attract new contributors

I fully stand behind ^^ - just look at the git history and how much Fabrice, Kenneth and Carsten (sorry if I've missed someone improved editing experience, cleanups and save actions in the last couple of releases. Both in terms of bug fixes to existing ones (like not losing code comments when applying a cleanup) and adding new such (whether we like it or not new users measure us by trying things like https://twitter.com/ZhekaKozlov/status/1135506701438857217 in Eclipse).
If we fail at attracting new users and contributors there would be more and more work on us (current committers) as the changes in environment only accelerate.
With the above said  - I fully understand this causes issues and that the duty to fix these ends up on current committers. I can definitely put myself in both positions - I still remember the endless crash story with SWT when distributions removed gnome-vfs (when and why I started contributing to SWT) and the fact that SWT codebase was just convoluted spaghetti plate so in order to fix a single issue I ended up changing test suites so they run locally, make it use Java 5 features, drop workarounds for Java 1.1 or J2ME and etc. and only after doing that I was able to fix a simple cut-off drawing of a text field. The reasons for that are 2 - most developers feel extremely discouraged when they are thrown into an unknown and ancient codebase and second by doing these "cleanups" one gains a significantly better understanding of the codebase.
I have introduced numerous issues by doing that but Silenio (mostly) and Bogdan handholded me and talked to me to help resolve the issues to the state when I was in their shoes and doing the same with Anatoly, Leo, Eric, Xi - the group of people that helped stabilize SWT (on GTK) to the point where things are still running OK even after they moved on to work on other things.
But people move on and we have to attract new contributors. This is the way we (people actively recruiting new contributors) know and the way we do it.
I would be more than happy if someone proves me wrong and manages to get the benefits without the (temporary) drawbacks.


 
, 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
_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev


--
Alexander Kurtakov
Red Hat Eclipse Team

Back to the top