Skip to main content

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

A clean structured code will help new comers to read it easily. I guess for JDT masters it might not make a difference since you have lived with the code and you might have written that code as well. Any code base deteriorate over time how much good you write it. So frequent cleanups and refactoring it essential. I this this problem have not been such a issue if object team had a proper ci pipeline building against latest JDT and these cleanups have been merged much earlier. 

Best regards,
Gayan

On Wed, 27 May 2020 at 20:54, Jeff Johnston <jjohnstn@xxxxxxxxxx> wrote:
Not me personally, but I have been reviewing a number of cleanup changes since I started as a JDT committer.  Carsten Hammer has contributed a large number of these.
That said, I have noticed that Carsten has started to test and is opening bugs, creating test cases, and has recently started providing the odd patch so he is
gradually dipping his toes into contributing more than just cleanups.

-- Jeff J.

On Wed, May 27, 2020 at 2:25 PM Kenneth Styrberg <kenneth@xxxxxxx> wrote:

The main point for me was to fix bugs. At first when I did my first patches I also removed all warnings from the code, but was encouraged not to do so to simplify reviews.

To do clean-ups as a starting point wasn't a thing for me.  Maybe a more junior programmer finds that more gratifying than me. My main motivation to continue was a responsive committer that actually took the time a review the patches and came with constructive comments. Even just a comment that he/she will look into it later, made it feel good, and that my time wasn't wasted.

I think doing clean-ups doesn't help ju understand the JDT code, you just follow a pattern without the need to know what the code actually do. Sure you learn how to setup your IDE connect to GIT and commit changes to Gerrit, that was also a blocker for which I had to reach out for help to solve, so it might be a first entry for some.

Regards,

Kenneth

Den 2020-05-27 kl. 19:46, skrev stephan.herrmann@xxxxxxxxx:

Anything in the order of "attracting contributors"?

Is it more attractive to start working on a component by making clean up changes? Do clean up changes help to get involved and serve as motivation to start working on functional changes, too?

Have you observed difficulties in understanding JDT code, that were resolved by doing a clean up change first? Examples?

thanks,
Stephan

Am 2020-05-27 16:40, schrieb 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



_______________________________________________
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