Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jdt-dev] "clean up" again 
Gayan, I am not discussing personal preferences of how code should look.

I'm discussing costs vs benefits.
I could put numbers to the costs, but I fail to see substantial benefits (i.e., benefits beyond personal taste) in some of the clean ups.
JDT does not live in a vacuum, it has some 20 years of history in the open, and this implies that this code is being accessed by plenty of code outside JDT. In this particular case I'm wearing both hats, and it pains me to spend lots of time on adjusting Object Teams after JDT clean ups, where this time could be better spent on fixing more bugs. 

Saying some class should actually be final is advice coming 20 years late.
If there's one thing to be learned from this: Every change in JDT potentially affects many other projects. 

best
Stephan

On 26.05.20 22:06, Gayan Perera wrote:

Hi All,
Good point, But at the same time i think the code change has no issue, i would say it should have made the class final as well looking at the commit message. But i do have another question, why does someone extend  a class which only have static methods ? Isn't that the main issue here ? So i would say the commiter should have made class final as well to make things more clear, and a downstream build should have found this issue sooner if we have such builds on the dependent projects. 

Best regards,
Gayan.
On Tue, May 26, 2020 at 8:55 PM Stephan Herrmann <stephan.herrmann@xxxxxxxxx <mailto:stephan.herrmann@xxxxxxxxx>> wrote: 

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

Back to the top