Re: Optional automated source code formatting - prep for JEP

[Basil Crow]

On Wed, Apr 12, 2023 at 6:06 AM 'jn...@cloudbees.com' via Jenkins
Developers  wrote:
> Please do not troll [sic] this out until 
> https://github.com/diffplug/spotless/issues/1559 is fixed

I am rejecting this request, as Spotless works fine on Windows in the
default configuration. Users of exotic configurations that are not
supported by Spotless on Windows should switch to the default
configuration or contribute support for their desired configuration to
Spotless rather than holding back others.

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAFwNDjrw-Fv5CS%2BpksWy%2BwfwZZ97KcTLyLL-0-mkao9ha0Xp_Q%40mail.gmail.com.

Re: Optional automated source code formatting - prep for JEP

['jn...@cloudbees.com' via Jenkins Developers]

> Please do not troll
that too but I meant roll this out.

On Wednesday, April 12, 2023 at 2:06:02 PM UTC+1 jn...@cloudbees.com wrote:

> Please do not troll this out until 
> https://github.com/diffplug/spotless/issues/1559 is fixed - this causes 
> all code to fail for windows users using checkout-as-is and checkin-as-is 
> which is not uncommon for windows users who also share code with the Linux 
> subsytem for windows.
>
> I have battled this for one repository where it was already enabled - and 
> to format the code you need to format everything then apply dos2unix on 
> everything (though should not be windows line ends - and some files may 
> need to be and have appropriate gitattributes).
>
> Rather than helping if the first thing a user sees is a lot of wanrings on 
> attempting to build what is just cloned - it will do the opposite.
>
> and before anyone suggests users should change their local environment 
> that is a non starter. - the projects should not require anything special 
> except git maven and java (at their supported/required versions)
>
> /James
> On Wednesday, April 12, 2023 at 6:08:15 AM UTC+1 m...@basilcrow.com wrote:
>
>> Thanks to everyone who has adopted the new Spotless configuration. I 
>> am trying to get everyone who was already using Spotless migrated over 
>> to the new configuration. Once that happens, I might even be able to 
>> change the opt-in mechanism from the ugly .mvn_exec_spotless file to a 
>> Maven property. 
>>
>> The next question is: do we want to do the same thing for core and 
>> core components? And if so, do we want it to be opt-in or shall we 
>> apply Spotless unconditionally for core and all core components? I can 
>> do the work, but it would take some time to cover all components. The 
>> question boils down to whether or not the other maintainers are 
>> comfortable with this change. 
>>
>> The last time we did a large formatting change in core (adding 
>> Prettier) the usual concerns were expressed regarding merge conflicts 
>> in open PRs and difficulty of backports. I think we did a pretty good 
>> job of resolving merge conflicts in open PRs, and we backported the 
>> formatting change in order to address concerns from the security team. 
>> We could adopt that (successful) strategy again in this case, assuming 
>> the same concerns still apply. 
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/30da2979-aaf0-4f48-bc2b-d3680004841fn%40googlegroups.com.

Re: Optional automated source code formatting - prep for JEP

['jn...@cloudbees.com' via Jenkins Developers]

Please do not troll this out 
until https://github.com/diffplug/spotless/issues/1559 is fixed - this 
causes all code to fail for windows users using checkout-as-is and 
checkin-as-is which is not uncommon for windows users who also share code 
with the Linux subsytem for windows.

I have battled this for one repository where it was already enabled - and 
to format the code you need to format everything then apply dos2unix on 
everything (though should not be windows line ends - and some files may 
need to be and have appropriate gitattributes).

Rather than helping if the first thing a user sees is a lot of wanrings on 
attempting to build what is just cloned - it will do the opposite.

and before anyone suggests users should change their local environment that 
is a non starter. - the projects should not require anything special except 
git maven and java (at their supported/required versions)

/James
On Wednesday, April 12, 2023 at 6:08:15 AM UTC+1 m...@basilcrow.com wrote:

> Thanks to everyone who has adopted the new Spotless configuration. I
> am trying to get everyone who was already using Spotless migrated over
> to the new configuration. Once that happens, I might even be able to
> change the opt-in mechanism from the ugly .mvn_exec_spotless file to a
> Maven property.
>
> The next question is: do we want to do the same thing for core and
> core components? And if so, do we want it to be opt-in or shall we
> apply Spotless unconditionally for core and all core components? I can
> do the work, but it would take some time to cover all components. The
> question boils down to whether or not the other maintainers are
> comfortable with this change.
>
> The last time we did a large formatting change in core (adding
> Prettier) the usual concerns were expressed regarding merge conflicts
> in open PRs and difficulty of backports. I think we did a pretty good
> job of resolving merge conflicts in open PRs, and we backported the
> formatting change in order to address concerns from the security team.
> We could adopt that (successful) strategy again in this case, assuming
> the same concerns still apply.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/8a2dd389-8287-4d02-8ed8-283590d8c1c6n%40googlegroups.com.

Re: Optional automated source code formatting - prep for JEP

[Basil Crow]

Thanks to everyone who has adopted the new Spotless configuration. I
am trying to get everyone who was already using Spotless migrated over
to the new configuration. Once that happens, I might even be able to
change the opt-in mechanism from the ugly .mvn_exec_spotless file to a
Maven property.

The next question is: do we want to do the same thing for core and
core components? And if so, do we want it to be opt-in or shall we
apply Spotless unconditionally for core and all core components? I can
do the work, but it would take some time to cover all components. The
question boils down to whether or not the other maintainers are
comfortable with this change.

The last time we did a large formatting change in core (adding
Prettier) the usual concerns were expressed regarding merge conflicts
in open PRs and difficulty of backports. I think we did a pretty good
job of resolving merge conflicts in open PRs, and we backported the
formatting change in order to address concerns from the security team.
We could adopt that (successful) strategy again in this case, assuming
the same concerns still apply.

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAFwNDjpOAQR88KOyxCYzdqE9h5-80qkugm1tnBXyex-nrmVrEA%40mail.gmail.com.

Re: Optional automated source code formatting - prep for JEP

[Basil Crow]

https://github.com/jenkinsci/plugin-pom/pull/733 is a non-draft PR
that incorporates all previous feedback and reflects practical
experience in the Jenkins and Maven projects.

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAFwNDjqQLd155%3DCx6YXg-k6TNn4voOqpj-puGGceHd4nOhMMbA%40mail.gmail.com.

Re: Optional automated source code formatting - prep for JEP

[Basil Crow]

On Thu, Feb 25, 2021 at 1:46 AM Liam Newman  wrote:
>
> Thanks you two, for clearly describing the issue that I'm attempting to
> address.

You're welcome! As I mentioned, I've done this a few times and have
learned what it takes to make the changes stick across a large number of
developers.

> Formatting issues will fail the build, source remains unchanged.
> Users may run "mvn spotless:apply" to automatically fix formatting.
> This command is easy and short.  When "spotless:check" fails it
> specifically tells users to run "mvn spotless:apply" to fix formatting.

This sounds great! +1 for Spotless. I have been using it for years and
have had nothing but positive experiences with the project and maintainer.

> Note: I am using the eclipse formatter rather than to google formatter

My recommendation is to use the Google Java formatter, but not because I
have any personal preference for the Google style. Rather my reasoning is
as follows. My experience with the Eclipse JDT formatter is that while it
can be made to format the majority of code at high quality, this requires
significant tweaking of the configuration options (as you have seen).
Tweaking opens up the door to endless subjective debates (as you have also
seen):

> If anyone disagrees with that evaluation, feel free to say so and
> explain why.

Having to spend time tweaking formatting configuration options means
increased development time. Having to spend time subjectively debating the
results of those tweaks in a PR increases review time. These outcomes
directly go against the stated goals from my previous message: to reduce
development time and to reduce review time.

While the Google Java formatter implies a more drastic transition, it
generally formats the majority of code at high quality without requiring
tweaking. Indeed, that it and Black are _not_ tweakable eliminates the
endless subjective debates that are typically associated with formatting
changes. In this way, it serves the two goals of reducing development time
and reducing review time.

A transition like this will be disruptive no matter which way you slice
and dice it. Better to bite the bullet and take most of the disruption up
front in order to maximize the long-term gain.

An anecdote: My company has been using the Google Java formatter since
2017, and most complaints stopped after 2018. Since 2019 nobody at my
company even thinks about formatting: the Google Java formatter is Good
Enough™ the majority of the time, it is consistently enforced, and it is
not up for debate. People focus on the logic, not the formatting, just as
intended. If you pick the Eclipse JDT formatter, you'll be focusing on
tweaking formatting configuration and subjectively debating formatting
preferences for years to come.

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAFwNDjp6p570_tW0icw5G8o%2Bgx11ufYrgJ%3DndmzQxcXXiFY%3DzA%40mail.gmail.com.

Re: Optional automated source code formatting - prep for JEP

[Liam Newman]

Thanks you two, for clearly describing the issue that I'm attempting to 
address.  
I've updated 
https://github.com/jenkinsci/github-branch-source-plugin/pull/392 with the 
following changes: 

   - Source will not be changed during the build unless the user runs a 
   specific maven goal
   - "spotless:check" runs near the end of the lifecycle during the verify 
  phase during local build.  Formatting issues will fail the build, source 
  remains unchanged. 
  - "spotless:check" runs near the start  of the lifecycle during the 
  process-sources phase during CI. Formatting issues will fail the build, 
  source remains unchanged. 
  - Users may run "mvn spotless:apply" to automatically fix 
  formatting.  This command is easy and short.  When "spotless:check" fails 
  it specifically tells users to run "mvn spotless:apply" to fix formatting.
   - I have adjusted the formatting rules to address some of the points 
   raised during review
   - Line comments do not join if wrapped - if you have comments that you 
  want to not be reformatted, keep them under 120 chars and use line 
comments
  - Javadoc formatting made a bit more compact 
  - 
https://github.com/jenkinsci/github-branch-source-plugin/pull/392/commits/ea6e28ca69ea24a59584fba3c6190b145dc0925e
  - Found a way to force array initializer expansion if that is what is 
  desired 
  - 
https://github.com/jenkinsci/github-branch-source-plugin/pull/392/commits/43a99508ab640db724e8b673c61855cb57670b82
  - Long string arguments getting pushed to the next line?  Maybe the 
  humans involved should split those 
  up. 
https://github.com/jenkinsci/github-branch-source-plugin/pull/392#discussion_r580403349
  - I've applied the ".gitattributes" suggested by Joseph.  The build 
  now passes on Windows.
   - I have also not addressed a number of other places where people have 
   said the formatting looks "worse". These cases are generally 
   debatable/personal-preference.  In some of these cases I agree with the 
   assessment but I don't see it as a blocker.  If anyone disagrees with that 
   evaluation, feel free to say so and explain why.  
  - 
https://github.com/jenkinsci/github-branch-source-plugin/pull/392#discussion_r580104724
 
  - 
  see 
https://github.com/bitwiseman/github-branch-source-plugin/pull/new/task/formatting-test-cleanup
  - 
https://github.com/jenkinsci/github-branch-source-plugin/pull/392#discussion_r580105388
 
  - same as previous
  - 
  
https://github.com/jenkinsci/github-branch-source-plugin/pull/392#discussion_r580402264
  - 
  
https://github.com/jenkinsci/github-branch-source-plugin/pull/392#discussion_r580510020
  - 
  
https://github.com/jenkinsci/github-branch-source-plugin/pull/392#discussion_r580513803
   
Note: I am using the eclipse formatter rather than to google formatter in 
an attempt to reduce the severity of the diff wall and disruption to folks 
familiar with the current code style. There are still significant 
differences, but they are far less than switching to google formatting.  
However, that is still an option. You can see what it would look like 
here: 
https://github.com/bitwiseman/github-branch-source-plugin/pull/new/task/formatting-google
 





On Wednesday, February 24, 2021 at 4:10:26 PM UTC-8 m...@basilcrow.com 
wrote:

> On Wed, Feb 24, 2021 at 3:32 AM jn...@cloudbees.com  
> wrote:
> >
> > What I feel is also missing here is what issue is this attempting to 
> solve.
> > It is proposing a solution - but what is the exact problem maintainers 
> are
> > seeing; perhaps there are better ways to solve that?
>
> The problem, to me, is twofold:
>
> 1. Lack of convenient formatting tools means one has to consciously think
> about formatting when writing new code or changing existing code. This
> slows down the development process.
> 2. Lack of a consistently enforced formatting style throughout a
> repository leads to PRs that change both logic and formatting style.
> This slows down the review process.
>
> Over the years I have led a number of efforts to reformat legacy codebases
> and enforce the new formatting style consistently at build time, both in
> Java (using Google Java Format) and Python (using Black). My experience is
> that while the "diff wall" described by Mark does initially lead to some
> frustration, the long term benefits of a consistently enforced formatting
> style eventually outweigh the initial costs. In my experience, the key
> takeaways are:
>
> 1. Choose a formatting tool that exposes extremely few (if any)
> configuration options and that formats the majority of code at high
> quality.
> 2. Provide developers with a single command (not executed by default) to
> automatically format their code prior to committing it (ideally also
> with IDE integration).
> 3. Enforce that the entire repository is consistently formatted during the
> build system's "verify" phase (

Re: Optional automated source code formatting - prep for JEP

[Basil Crow]

On Wed, Feb 24, 2021 at 3:32 AM jn...@cloudbees.com  wrote:
>
> What I feel is also missing here is what issue is this attempting to solve.
> It is proposing a solution - but what is the exact problem maintainers are
> seeing; perhaps there are better ways to solve that?

The problem, to me, is twofold:

1. Lack of convenient formatting tools means one has to consciously think
   about formatting when writing new code or changing existing code. This
   slows down the development process.
2. Lack of a consistently enforced formatting style throughout a
   repository leads to PRs that change both logic and formatting style.
   This slows down the review process.

Over the years I have led a number of efforts to reformat legacy codebases
and enforce the new formatting style consistently at build time, both in
Java (using Google Java Format) and Python (using Black). My experience is
that while the "diff wall" described by Mark does initially lead to some
frustration, the long term benefits of a consistently enforced formatting
style eventually outweigh the initial costs. In my experience, the key
takeaways are:

1. Choose a formatting tool that exposes extremely few (if any)
   configuration options and that formats the majority of code at high
   quality.
2. Provide developers with a single command (not executed by default) to
   automatically format their code prior to committing it (ideally also
   with IDE integration).
3. Enforce that the entire repository is consistently formatted during the
   build system's "verify" phase (Maven) or "check" task (Gradle), but do
   not change any existing code as part of this process.

This solves the originally mentioned problem as follows:

1. A formatting tool that is integrated with the build system means that
   one no longer has to consciously think about formatting when writing
   new code or changing existing code. This speeds up the development
   process.
2. A consistently enforced formatting style throughout a repository
   eliminates PRs that change both logic and formatting style. There is
   only one formatting style, and that is the style imposed by the
   formatting tool during the build (otherwise the build is unstable).
   This speeds up the review process.

Some practical notes:

1. The Eclipse JDT formatter is very customizable, but customizability is
   a non-goal when trying to enforce a consistent formatting style across
   an entire repository.
2. Google Java Format [1] and Black [2] are intentionally not customizable
   and format the majority of code at high quality.
3. One exception to the above with Google Java Format is that it supports
   an "AOSP mode" to switch from 2-space indentation to 4-space
   indentation. One might be tempted to use this mode when formatting a
   legacy codebase that already uses 4-space indentation; however, the
   result is often unreadable lambda expressions [3], and the maintainers
   do not appear to be willing to address this. I highly recommend
   avoiding this "AOSP mode" and using the default (2-space indentation)
   mode when formatting a codebase with Google Java Format.

[1] https://github.com/google/google-java-format/wiki/FAQ
[2] https://github.com/psf/black#the-black-code-style
[3] https://github.com/google/google-java-format/issues/19

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAFwNDjpDaim1x8X8YP70J1B1aJwCNUe4GKGKxq85cyOrcy18Tg%40mail.gmail.com.

Re: Optional automated source code formatting - prep for JEP

[Mark Waite]

On Wednesday, February 24, 2021 at 4:32:23 AM UTC-7 James wrote:

> Hi all,
>
> So I have some strong opinions here and these are that nothing should ever 
> be auto applied.
>
> Whatever format you have defined it will produce worse code in some 
> situations, and this results in code littered with 
> //AUTO_FORMAT_IGNORE:next_10_lines which does not aid to the readability of 
> the code or code that is just harder to read than it should be.
>
> I have found that what works best is if there is a way for users to format 
> my changes "on demand" by say running a specific maven goal, or having a 
> specific IDE configuration (not all IDEs can format the same way so this 
> last one is hard) , *and *for the CI to flag deviations as part of the 
> code review.  ie it should not fail the build, but say "this line violates 
> rule line_length > X".  That way as a maintainer I can choose to accept 
> something that I believe is legible when the committer has left a comment 
> saying why the code deviates, or just leave it as the CI saying "please fix 
> your style issues".
>
> There is also nothing worse than running `mvn -Dmaven-surefire-debug=XYZ 
> test -Dtest-foobar` and having all the breakpoints messed up because the 
> source code changes underneath you, or running tests from your IDE, 
> committing and then pushing only for the CI to fail with something that is 
> probably not easy to see (exit code 1 without any CI check comment?)
>
> What I feel is also missing here is what issue is this attempting to 
> solve.  It is proposing a solution - but what is the exact problem 
> maintainers are seeing; perhaps there are better ways to solve that?
>
>
I was trying to avoid changes in indentation, braces, and comments that I 
felt did not add value to git plugin and git client plugin pull requests.  
I was spending time reminding submitters that the contributing guide in 
those plugins specifically asked that they not make white space changes 
without consulting the maintainers.  I was occasionally frustrated when I 
failed to follow the contributing guidelines for that plugin in my 
reviews.  Then white space changes would arrive in the code even though the 
guidelines said they should not.  Worrying about formatting during code 
review seemed like a waste of time.  Two years ago the git plugin had over 
60 pull requests open proposing various changes.  A white space or 
formatting change in one of those pull requests would cause another pull 
request to have a conflict.

Since that time, I've resolved the issue in the git plugin another way.  I 
declared pull request review bankruptcy and closed 50+ pull requests 
because I was clearly not making progress reviewing them.

Since then, I've implemented mandatory code formatting in another plugin 
(platformlabeler) as an experiment and have found the code formatting 
helpful.  The one open pull request on that repository has been complicated 
by a "diff wall" created by a change from two space indentation to four 
space indentation, but that diff wall was not difficult to handle in that 
case.

I still remember with affection the experience watching a team adopt 
automated formatting of their Java code just because Kent Beck's Extreme 
Programming book said to do it.  We were feeling bold and wanted to go "all 
in".  It removed one area of friction and allowed them to work better 
together.  I suspect the `go fmt` command is similarly used to reduce 
friction due to formatting styles.

I've accepted that the git plugin and git client plugin will likely never 
use automated formatting because of the "diff wall" it would create.  
Others have described techniques to avoid or manage the "diff wall", but 
the formatting in those two plugins is such a mess that I believe the "diff 
wall" is unavoidable in their cases.

Mark Waite

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/8028b729-05a8-43d2-bd9f-05460b188f94n%40googlegroups.com.

Re: Optional automated source code formatting - prep for JEP

[jn...@cloudbees.com]

Hi all,

So I have some strong opinions here and these are that nothing should ever 
be auto applied.

Whatever format you have defined it will produce worse code in some 
situations, and this results in code littered with 
//AUTO_FORMAT_IGNORE:next_10_lines which does not aid to the readability of 
the code or code that is just harder to read than it should be.

I have found that what works best is if there is a way for users to format 
my changes "on demand" by say running a specific maven goal, or having a 
specific IDE configuration (not all IDEs can format the same way so this 
last one is hard) , *and *for the CI to flag deviations as part of the code 
review.  ie it should not fail the build, but say "this line violates rule 
line_length > X".  That way as a maintainer I can choose to accept 
something that I believe is legible when the committer has left a comment 
saying why the code deviates, or just leave it as the CI saying "please fix 
your style issues".

There is also nothing worse than running `mvn -Dmaven-surefire-debug=XYZ 
test -Dtest-foobar` and having all the breakpoints messed up because the 
source code changes underneath you, or running tests from your IDE, 
committing and then pushing only for the CI to fail with something that is 
probably not easy to see (exit code 1 without any CI check comment?)

What I feel is also missing here is what issue is this attempting to 
solve.  It is proposing a solution - but what is the exact problem 
maintainers are seeing; perhaps there are better ways to solve that?

/James







On Tuesday, February 23, 2021 at 2:00:05 PM UTC bma...@gmail.com wrote:

> Thinking about this more, I agree the right phase should be on 
> both process-resources and process-test-resources respectively (though a 
> bit surprising for users indeed).
>
> On the way it should operate: I was going to propose a CI-only flag to 
> fail there when a format is not good, but to avoid the CI surprise, I'm 
> thinking the standard build should:
> * Detect a formatting change need, do it, and non-zero exit
> Or
> * Detect no formatting need and do nothing
>
> This way the behavior is the same locally and in Jenkins, and it should 
> not be too disruptive as it's not very late in the Maven lifecycle.
>
> I think non-zero exiting is useful locally, so you don't end up git add & 
> commit, mvn test, git push but then get a CI failure because you had a 
> local formatting change you didn't add before pushing.
>
> Le mar. 24 juil. 2018 à 22:44, Liam Newman  a écrit :
>
>> The auto format should occur before the compile stage, probably during 
>> process-sources
>
>
>>
>> On Tue, Jul 24, 2018, 6:00 AM Baptiste Mathus  wrote:
>>
>>> Great idea and initiative IMO, thanks a bunch for driving this Mark. 
>>>
>>> Over the years, I've become more and more strongly opinionated that *a* 
>>> common format is defined, and easily applied, and less and less about the 
>>> precise format.
>>> IOW, I generally do have opinions about where curly brackets and others 
>>> should go, but I'll happily put them aside if this helps us define 
>>> *something*.
>>>
>>> @Mark you can use https://github.com/jenkinsci/buildtriggerbadge-plugin 
>>> or https://github.com/jenkinsci/essentials-plugin and few others to 
>>> test drive this. 
>>> https://github.com/jenkinsci/radiatorview-plugin might also be 
>>> interesting as it has a bit more code, and has been existing for quite long 
>>> I think.
>>>
>>> -- Baptiste
>>>
>>>
>>>
>>> Le mar. 24 juil. 2018 à 03:36, Mark Waite  a 
>>> écrit :
>>>
 I'd like to submit a Jenkins Enhancement Proposal for optional 
 automated source code formatting available from the plugin pom so that 
 plugins which build with maven could choose to "opt-in" to automated 
 source 
 code formatting.

 Key attributes of the idea:

- *Optional* - plugins only get source code formatting if they 
enable it
- *Automatic* - when source code formatting is enabled, it happens 
automatically as part of the maven compile phase
- *Common* - when source code formatting is enabled, the source 
code formatting settings are not intended to be altered by the plugin

 I've implemented a sample in the platformlabeler plugin 
 
  
 using the "FMT Maven plugin 
 " which 
 uses google-java-format  
 to format Java source code to Google Java Style 
 .

 Key questions that need discussion (especially considering my feeble 
 skills with maven):

- Is optional automated source formatting of interest to a few 
plugin developers that would be willing to test drive it?  If so, who 
 are 

Re: Optional automated source code formatting - prep for JEP

[Baptiste Mathus]

Thinking about this more, I agree the right phase should be on
both process-resources and process-test-resources respectively (though a
bit surprising for users indeed).

On the way it should operate: I was going to propose a CI-only flag to fail
there when a format is not good, but to avoid the CI surprise, I'm thinking
the standard build should:
* Detect a formatting change need, do it, and non-zero exit
Or
* Detect no formatting need and do nothing

This way the behavior is the same locally and in Jenkins, and it should not
be too disruptive as it's not very late in the Maven lifecycle.

I think non-zero exiting is useful locally, so you don't end up git add &
commit, mvn test, git push but then get a CI failure because you had a
local formatting change you didn't add before pushing.

Le mar. 24 juil. 2018 à 22:44, Liam Newman  a écrit :

> The auto format should occur before the compile stage, probably during
> process-sources


>
>
>
> On Tue, Jul 24, 2018, 6:00 AM Baptiste Mathus  wrote:
>
>> Great idea and initiative IMO, thanks a bunch for driving this Mark.
>>
>> Over the years, I've become more and more strongly opinionated that *a*
>> common format is defined, and easily applied, and less and less about the
>> precise format.
>> IOW, I generally do have opinions about where curly brackets and others
>> should go, but I'll happily put them aside if this helps us define
>> *something*.
>>
>> @Mark you can use https://github.com/jenkinsci/buildtriggerbadge-plugin
>> or https://github.com/jenkinsci/essentials-plugin and few others to test
>> drive this.
>> https://github.com/jenkinsci/radiatorview-plugin might also be
>> interesting as it has a bit more code, and has been existing for quite long
>> I think.
>>
>> -- Baptiste
>>
>>
>>
>> Le mar. 24 juil. 2018 à 03:36, Mark Waite  a
>> écrit :
>>
>>> I'd like to submit a Jenkins Enhancement Proposal for optional automated
>>> source code formatting available from the plugin pom so that plugins which
>>> build with maven could choose to "opt-in" to automated source code
>>> formatting.
>>>
>>> Key attributes of the idea:
>>>
>>>- *Optional* - plugins only get source code formatting if they
>>>enable it
>>>- *Automatic* - when source code formatting is enabled, it happens
>>>automatically as part of the maven compile phase
>>>- *Common* - when source code formatting is enabled, the source code
>>>formatting settings are not intended to be altered by the plugin
>>>
>>> I've implemented a sample in the platformlabeler plugin
>>> 
>>> using the "FMT Maven plugin
>>> " which
>>> uses google-java-format 
>>> to format Java source code to Google Java Style
>>> .
>>>
>>> Key questions that need discussion (especially considering my feeble
>>> skills with maven):
>>>
>>>- Is optional automated source formatting of interest to a few
>>>plugin developers that would be willing to test drive it?  If so, who are
>>>the plugin developers that are willing to test drive it, and which 
>>> plugins
>>>will be involved in the test drive?  I'll use platformlabeler as one of 
>>> the
>>>test drive plugins.  Others will be needed
>>>- How would a prototype of optional automated source formatting be
>>>evaluated?  By releasing a beta version of the plugin parent pom
>>>
>>> ?
>>>Some other means?
>>>- Would automated formatting be integrated into the plugin parent
>>>pom using techniques similar to the findbugs integration
>>>
>>> 
>>>into the plugin parent pom, or is there a better way?
>>>- Is "google java format" an acceptable code format for those plugin
>>>authors that are willing to adopt automated code formatting?
>>>- Are there other issues that need to be addressed before a Jenkins
>>>Enhancement Proposal is submitted for optional automated source code
>>>formatting?
>>>
>>> A plugin maintainer that chooses to adopt automated source code
>>> formatting needs to understand that automated source code formatting
>>> creates a "diff wall" due to the many changes made by the formatting
>>> process.  That is one of the penalties associated with automated source
>>> code formatting.
>>>
>>> Plugins with many open pull requests (git plugin, git client plugin)
>>> should not enable automated formatting until they've reduced the number of
>>> open pull requests to very few.  Otherwise, most open pull requests for the
>>> plugin will have many conflicts as soon as the automated formatting is
>>> committed.  That is anoth

Re: Optional automated source code formatting - prep for JEP

[Jesse Glick]

An ongoing attempt:
https://github.com/jenkinsci/github-branch-source-plugin/pull/392

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr0WLXn6-EGsUQDKE2%3DSc2xd_wYazBvoqTiJiHuAgE0S3A%40mail.gmail.com.

Re: Optional automated source code formatting - prep for JEP

[Joseph P]

I just realized this is the minimum

*.java eol=lf

However this would be could to have in all repos:
* text=auto
*.java eol=lf

onsdag den 25. juli 2018 kl. 14.26.22 UTC+2 skrev Joseph P:
>
> The action needed to avoid line endings being messed with is 
> * text=auto
>
> However this would be enough for the formatter
> *.java text eol=lf
>
> But I would recommend the text=auto
>
> And if the maintainer needs something more fancy they can customise it 
> further but lucky shouldn't have anything like Visual Studio
> *.slntext eol=crlf
>
> If a maintainer mess it up the answer would be to add the .gitattributes 
> and run 
> git add --renormalize .
>
> Like I did in the first PR.
>
> onsdag den 25. juli 2018 kl. 14.06.31 UTC+2 skrev Mark Waite:
>>
>>
>>
>> On Wed, Jul 25, 2018 at 4:44 AM Joseph P  wrote:
>>
>>> Just to touch on the line endings issue... See 
>>> https://github.com/jenkinsci/vstestrunner-plugin/pull/16 when it 
>>> becomes an issue. I just wanted to fix a minor thing but because of the 
>>> maintainer did a commit where they messed with line endings it screwed up 
>>> the history for https://github.com/jenkinsci/vstestrunner-plugin/pull/17
>>>
>>>
>> I see the changes in line endings between those two commits but am not 
>> clear on the actions which are needed by a maintainer to avoid the problem.
>>
>> Is it enough to have the .gitattributes file that sets the line endings 
>> of files that will be automatically formatted?
>>
>> If that's not enough, could you explain further what is needed?
>>
>> Mark Waite
>>  
>>
>>> tirsdag den 24. juli 2018 kl. 14.30.01 UTC+2 skrev Mark Waite:

 On Tue, Jul 24, 2018 at 1:48 AM Joseph P  wrote:

> The problem I have noticed with FMT Maven plugin. The formatter does 
> not handle CR LF line endings rather it converts everything to LF which 
> will mess with commits because it causes high rewrites. So you would need 
> to ensure that git attributes is in all repos as well to avoid incorrect 
> commit history.
>
> Tomas Bjerre is using it on his repos: 
> https://github.com/jenkinsci/violation-comments-to-stash-plugin
> See this commit message: 
> https://github.com/jenkinsci/violation-comments-to-stash-plugin/pull/50/commits/1f2fd46632aaf65493f41bf74e660710221fd70f
>
>
 I like the idea of including a .gitattributes file as part of the 
 transition to optional automatic formatting in a repository.  I'll include 
 that in the proposal.  Additional files were needed with the transition to 
 support incrementals and that transition seems to be progressing very well.

 I prefer (and accept) the intentional setting of line endings.  I 
 consider that part of the standardization of the formatting.  I think that 
 maintainers that don't want line endings standardized or that cannot 
 accept 
 LF as the standard would choose to not enable automatic formatting in the 
 plugins they maintain.

 Thanks,
 Mark Waite
  

>>>
> tirsdag den 24. juli 2018 kl. 03.36.42 UTC+2 skrev Mark Waite:
>>
>> I'd like to submit a Jenkins Enhancement Proposal for optional 
>> automated source code formatting available from the plugin pom so that 
>> plugins which build with maven could choose to "opt-in" to automated 
>> source 
>> code formatting.
>>
>> Key attributes of the idea:
>>
>>- *Optional* - plugins only get source code formatting if they 
>>enable it
>>- *Automatic* - when source code formatting is enabled, it 
>>happens automatically as part of the maven compile phase
>>- *Common* - when source code formatting is enabled, the source 
>>code formatting settings are not intended to be altered by the plugin
>>
>> I've implemented a sample in the platformlabeler plugin 
>> 
>>  
>> using the "FMT Maven plugin 
>> " 
>> which uses google-java-format 
>>  to format Java source 
>> code to Google Java Style 
>> .
>>
>> Key questions that need discussion (especially considering my feeble 
>> skills with maven):
>>
>>- Is optional automated source formatting of interest to a few 
>>plugin developers that would be willing to test drive it?  If so, who 
>> are 
>>the plugin developers that are willing to test drive it, and which 
>> plugins 
>>will be involved in the test drive?  I'll use platformlabeler as one 
>> of the 
>>test drive plugins.  Others will be needed
>>- How would a prototype of optional automated source formatting 
>>be evaluated?  By releasing a beta version o