Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [papyrus-rt-dev] Code formatting and checkstyle rules

Hi, Peter,

With the EOL clean-up there really is no choice but to do all files at once, because any single file that has EOL errors blocks all sorts of critical git operations.

It’s different with formatting.  I don’t see much value in changing files that don’t need functionally meaningful changes just to update the formatting.  But, that will be up to the committer team to decide.  In any case, if you are concerned about what the save actions do, there is a remedy:  everything done by the save action is implemented as an undoable operation.  After hitting Cmd+S to save, just hit Cmd+Z to undo what the save actions did, reverting the file to just the changes that you intentionally made.  So, for example, you could make some edits, hit Cmd+Shft+O to organize imports, Cmd+S to save, then Cmd+Z to revert to your edits with imports.

HTH,

Christian

On 17 March, 2016 at 04:46:59, Peter Cigéhn (peter.cigehn@xxxxxxxxx) wrote:

Hi,

If I may jump in, as a beginner contributor, I would personally prefer a one-time update, in the corresponding was as Christian did with the recent EOL cleanup.

As a beginner contributor, I prefer to change as little as possible in the contributions I make, to feel more confident that I do not mess things up. I have a feeling that if the automatic layout happens for more or less a complete file (in the corresponding way as when you have EOL issues), it might be harder for me (and the one doing the review) to see/understand that I don't change more than needed.

But maybe this is an exaggerated concern.

/Peter Cigéhn

On 17 March 2016 at 09:39, SCHNEKENBURGER Remi 211865 <Remi.SCHNEKENBURGER@xxxxxx> wrote:

Hi Christian,

 

I am very happy with the first gerrit patch! I will wait for more feedback from other committers. We could then merge this initial contribution if no one has any complaint.

 

I have no strong opinion about doing an all-in-one update on format or waiting for a touch on each file by commiters.

 

Regards,

Rémi

 

 

-------------------------------------------------------

 

Rémi SCHNEKENBURGER

+33 (0)1 69 08 48 48

CEA Saclay Nano-INNOV

Institut CARNOT CEA LIST

 

Description : PapyrusLogo_SmallFormatwww.eclipse.org/papyrus

 

De : papyrus-rt-dev-bounces@xxxxxxxxxxx [mailto:papyrus-rt-dev-bounces@xxxxxxxxxxx] De la part de Christian Damus
Envoyé : mercredi 16 mars 2016 20:25


À : papyrus-rt developer discussions <papyrus-rt-dev@xxxxxxxxxxx>
Objet : Re: [papyrus-rt-dev] Code formatting and checkstyle rules

 

Hi, all,

 

I have raised Bug 489782 to track our efforts on this subject and provided a Gerrit patch that implements what I proposed below.  Note that the patch doesn’t do anything for Checkstyle configuration, yet.  That can follow once we’ve decided what that should look like.  Hopefully we can iterate on this patch to get something we like and continue the discussion in the bugzilla.

 

Christian

 

 

 

On 16 March, 2016 at 14:42:16, Christian Damus (give.a.damus@xxxxxxxxx) wrote:

Hi, Rémi,

I am satisfied with the Papyrus code formatter. It is very much like the Java conventions.

For save actions, I recommend

  • Format source code
  • with the option to format all lines in the file, not just edited lines
  • note that the save action uses the formatter profile configured in the project settings
  • I would not recommend formatting the world and pushing a commit that normalizes all of the formatting in the repo. Just let it happen via the save action as developers edit each file that needs changes anyways
  • Organize imports
  • the organize-imports settings for ordering and grouping should be shared in the source projects so that everybody gets the same results
  • Additional actions:
  • add missing @Override
  • add missing @Deprecated
  • Remove unnecessary casts

In the Papyrus project, Oomph manages these settings for all projects:

  • JDT core settings managed by the build path EE selection
  • compiler source/binary compliance level and codegen target platform
  • JDT Core settings specified in the org.eclipse.papyrus.infra.core project:
  • the formatter settings from the Papyrus Code Formatter Profile
  • JDT UI settings specified in the org.eclipse.papyrus.infra.core project:
  • code clean-up settings from the Papyrus Code Clean-up Profile
  • organize-imports namespace order: java,javax,org,com
  • code templates from the XML file in the Papyrus repo developer documents (including the copyright header)

The Papyrus project doesn’t enforce any Save Actions. I wish it would, and would recommend the same for Papyrus-RT.

These settings are mastered in the org.eclipse.papyrus.infra.core project and are pushed from it to all other projects in the same repository that you have open in your workspace. That is, if you have the Oomph Project Configuration tooling installed (which you would if you used the Papyrus.setup). But, because every project has the settings checked in to git, they are in effect for everybody that uses JDT, whether they have the Oomph tools installed or not. What the Oomph tools do is to make sure that all projects have the correct settings in the first place.

Cheers,

Christian

 

On 16 March, 2016 at 13:55:41, SCHNEKENBURGER Remi 211865 (remi.schnekenburger@xxxxxx) wrote:

Hi all,

 

Thanks to everyone for this fruitful discussion! I could see here a lot of proposals and volunteers ;)

 

I would propose the following things:

-       Papyrus-RT should reuse the Papyrus default formatter, as this would reduce work when ‘upgrading’ some code from Papyrus-RT to generic Papyrus. It is also very similar to default formatter AFAIR.

-       Papyrus-RT should reuse a checkstyle configuration close to a generic configuration, as for example the one in Sonar. I would prefer not to get too far from existing configurations, and a simple one to begin with. There is already a basic one within Eclipse. Céline proposes one, is there one on Collaborative modeling, Philip?

-       The save action could format the code and do the basic cleaning (import cleaning, etc.)

 

I like very much the idea of pre-commit hooks, because usually the only respected rules are the one that are enforced ;) but I don’t know here how it does fit well with Eclipse, Egit & command line users, etc. I would prefer to avoid manual actions to be performed by developers to setup the environment : Oomph was typically invented to reduce the amount of time needed to start committing on a project while removing manual tasks.

 

To have a concrete plan:

-       @Céline, can you provide a report on current Papyrus-RT code base with a basic checkstyle configuration or the one you usually use?

-       @Philip, can you propose also a checkstyle configuration file from collaborative modeling?

-       @Céline, can you give an update to all committers on the status of the Sonar analysis job on Papyrus-RT? Is this possible to run this analysis on each gerrit contributions?

-       @All, what actions should be done on Save?

-       @All, do you agree to reuse Papyrus formatter?

-       @Christian, which settings would you propose from the Papyrus project on the project .settings - JDT settings and others?

 

Thanks,

Rémi

 

 

-------------------------------------------------------

 

Rémi SCHNEKENBURGER

+33 (0)1 69 08 48 48

CEA Saclay Nano-INNOV

Institut CARNOT CEA LIST

 

Description : PapyrusLogo_SmallFormatwww.eclipse.org/papyrus

 

De :papyrus-rt-dev-bounces@xxxxxxxxxxx [mailto:papyrus-rt-dev-bounces@xxxxxxxxxxx] De la part de Ernesto Posse
Envoyé : mercredi 16 mars 2016 18:14
À : papyrus-rt developer discussions <papyrus-rt-dev@xxxxxxxxxxx>
Objet : Re: [papyrus-rt-dev] Code formatting and checkstyle rules

 

So we should set the "Save Actions" individually and commit those to the repo?

 

 

On Wed, Mar 16, 2016 at 12:49 PM, Christian Damus <give.a.damus@xxxxxxxxx> wrote:

Yes, all of the JDT preferences can be managed per project in the .settings folder.

 

Christian


_______________________________________________
papyrus-rt-dev mailing list
papyrus-rt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/papyrus-rt-dev



 

--

Ernesto Posse

Zeligsoft.com

 

_______________________________________________
papyrus-rt-dev mailing list
papyrus-rt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/papyrus-rt-dev


_______________________________________________
papyrus-rt-dev mailing list
papyrus-rt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/papyrus-rt-dev


_______________________________________________
papyrus-rt-dev mailing list
papyrus-rt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/papyrus-rt-dev

Back to the top