| Re: [jdt-dev] "clean up" again |
Hi all,> We have the relevant people with experience of working on JDT code right here in
> the list. Let's use that opportunity for learning based on specific
> observations, rather than general statements that could also be found in a text
> book.I've not done that much in JDT. But with my limited experience and knowledge, I would not consider doing any sort of style changes in JDT code (code style is subjective anyway). For clean-ups, I would at most consider indentations, as in some cases the save actions "fight" with changes. Doing an automatic tool change in 10s of files (if not 100s) is absolutely out of the question for me. Especially having witnessed enough issues that have slipped through the tests, due to at first glance harmless changes.My reasoning: Very often a piece of code in JDT has been untouched for years, in some cases over 10 years. Even in a code base that I'm deeply familiar with, and maintain on a daily basis, I would make minimal (bug-fix only) changes to such code.I can't make a comment on attracting other contributors in JDT.
I would image a developer contributes to JDT out of necessity; something doesn't work or a feature is needed.
For the git history point that was brought up, two items have been very frustrating for me:1. The code has essentially no git history, as it has been untouched since the first addition to the git repository. Obviously not much that can be improved here.2. The top commits are indentation / style change / a revert thereof. This breaks the history annotations and often I have to resort to gitk to quickly find the origin of a line.Also please reconsider having a bug for every change (not just in JDT). Its a bit more work now, but saves a lot of time later down the line. Its IMO the best practice I've seen in Eclipse development.Best regards,Simeon_______________________________________________On Thu, 28 May 2020 at 12:53, Stephan Herrmann <stephan.herrmann@xxxxxxxxx> wrote:I think we need to distinguish two things:
A. Adjusting the code structure to changing requirements to work against
architectural decay.
B. Mechanical transformations, e.g., in order to use more modern syntax.
We typically consider (A) during *every* code change, at least we should.
Sometimes, a quick band-aid is committed and a follow-up ticket filed, to iron
out a kludge.
Recovering design intentions that have been lost during evolution is one of the
hardest tasks, typically involving a lot of time reading through git history and
lots of bugs ("software archeology"), and then designing & documenting a better
structure. Such changes require review - if not participation - of a senior
committer.
I'm not sure how changes in category (B) could possibly work against "code base
deteriorating over time".
So rather than speculate, show me an example, where code was hard to read and is
significantly easier to read after clean-up. Is improvement in the order of
saving one or two seconds here and there, or can you show examples where the
change caused "now I understand what this code is doing"?
We have the relevant people with experience of working on JDT code right here in
the list. Let's use that opportunity for learning based on specific
observations, rather than general statements that could also be found in a text
book.
best,
Stephan
On 28.05.20 08:00, Gayan Perera wrote:
> 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
> <mailto: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
> <mailto: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
> <mailto: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
>> <mailto: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
>>> <mailto: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
>>> <mailto: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 <mailto:jdt-dev@xxxxxxxxxxx>
>>> To unsubscribe from this list, visit
>>> https://www.eclipse.org/mailman/listinfo/jdt-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> jdt-dev mailing list
>>> jdt-dev@xxxxxxxxxxx <mailto:jdt-dev@xxxxxxxxxxx>
>>> To unsubscribe from this list, visit
>>> https://www.eclipse.org/mailman/listinfo/jdt-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> jdt-dev mailing list
>>> jdt-dev@xxxxxxxxxxx <mailto:jdt-dev@xxxxxxxxxxx>
>>> To unsubscribe from this list, visit
>>> https://www.eclipse.org/mailman/listinfo/jdt-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> jdt-dev mailing list
>>> jdt-dev@xxxxxxxxxxx <mailto:jdt-dev@xxxxxxxxxxx>
>>> To unsubscribe from this list, visit
>>> https://www.eclipse.org/mailman/listinfo/jdt-dev
>>
>>
>>
>> _______________________________________________
>> jdt-dev mailing list
>> jdt-dev@xxxxxxxxxxx <mailto:jdt-dev@xxxxxxxxxxx>
>> To unsubscribe from this list, visithttps://www.eclipse.org/mailman/listinfo/jdt-dev
> _______________________________________________
> jdt-dev mailing list
> jdt-dev@xxxxxxxxxxx <mailto:jdt-dev@xxxxxxxxxxx>
> To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/jdt-dev
>
> _______________________________________________
> jdt-dev mailing list
> jdt-dev@xxxxxxxxxxx <mailto: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