--
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-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/c3ef8d20-8332-47f6-9803-68178421aac0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
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
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 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 another of the penalties of automated source code formatting.Plugins that adopt automated source code formatting will likely find it easier to read the source code and easier to submit clean pull requests. Automated formatting avoids the poor experience of pull request submitters being asked to fix the white space diffs introduced by their pull request.Thanks,Mark Waite
--
I am more of the philosophy of "teach a man to code and he will eat for a lifetime" approach. So most of my "originally authored" plugins have checkstyle rules applied to the build.It is by no means perfect, but I haven't had to complain about white spaces in PRs to those plugins in a long time :)/B
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CALzHZS2e5mS4EN4AzCRVhVa0-nsT2B7t47QeMDMBTFPGgwyjaw%40mail.gmail.com.
--
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-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAO49JtExx4tXNbAVCSr85jPDTKR1Zy_BJrxkcp_kJdAfcVaZgQ%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS4oBgvoAZnWDh8Fw9HfNATZFcrg3ZDKm4m0LwLYWti7Ew%40mail.gmail.com.
On Mon, Jul 23, 2018 at 9:36 PM Mark Waite <mark.ea...@gmail.com> wrote:
> Automatic - when source code formatting is enabled, it happens automatically as part of the maven compile phase
Meaning that `mvn compile` modifies `src/main/java/**/*.java`?
Convenient if potentially surprising.
But what about pull requests in which the author never ran Maven to
this goal, and edited sources do not match the format—is there any way
for the PR build to fail in this case? (Note that this does _not_ mean
the PR author never tested/tried the code. Some IDEs run internal
compilation, bypassing Maven.)
> How would a prototype of optional automated source formatting be evaluated? By releasing a beta version of the plugin parent pom?
Just by doing a regular release of the plugin parent POM. If the
change is a dud, we just back that configuration out in a later
release.
> Is "google java format" an acceptable code format for those plugin authors that are willing to adopt automated code formatting?
BTW I am leery of automated source formatting generally, but if I have
to comply with a format, it is nice that it allows inline
determinative annotations such as `@Override` or `@Test` without
someone yelling at me about some arbitrary Sun Microsystems-era
guideline. :-) It remains unfortunate that it prohibits inline
annotations with any parameters even when they are determiners, like
the second line in
@Parameterized.Parameter public String x;
@Parameterized.Parameters(name = "x={0}") public static String[] allowedXs() {
// …
}
since inserting a line break would make `Parameters` look like a mere
attribute, which it is not.
My heart was also warmed by this note:
> Never make your code less readable simply out of fear that some programs might not handle non-ASCII characters properly. If that should happen, those programs are broken and they must be fixed.
--
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-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr1udMOYvASdbT5LOn_Se8bOtmzCxx9Y53T6zG32fCpuxA%40mail.gmail.com.
--
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-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr1W8ZYb6_R67b55pcdke-CZHwKkkfr%2BnexW569YfkhJSA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Or maybe that just gets pushed together with the "[maven-release-plugin] prepare release ..." commit?
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
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/a78c887f-e3ed-4d8a-a750-9ea71865c9a5%40googlegroups.com.
* text=auto
*.java text eol=lf
*.sln text eol=crlf
git add --renormalize .
--
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-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr18En5T-JF%2BOk4tc_%3D-CTp1BNQVA%3DHT6x5jvddw5vk8_w%40mail.gmail.com.
On Wed, Jul 25, 2018 at 9:40 AM Mark Waite <mark.ea...@gmail.com> wrote:
> I think your observation means that you would probably not choose to enable automatic source formatting in the plugins you maintain if the formatting is "always on".
No, I am referring to _contributing_ to such a plugin maintained by
someone else. I would not expect a `mvn` command in the default
lifecycle to ever modify a versioned file. That generally counts as a
serious bug in the build setup.
> Would it meet your needs if there were two modes of operation, one that includes a fmt-maven-plugin:check in the process-sources phase without modifying source code, and one that performs the formatting in the process-sources phase?
There are already two modes of operation being discussed. The question
is what happens by default when I clone somebody’s repo, edit a couple
of lines, run tests from my IDE, commit, maybe do some follow-up Maven
build, and file a PR against it. Am I suddenly left with uncommitted
modifications in my source tree?
Worse, what happens when I edit some lines, instruct my IDE to run a
Maven build to run tests (saving all modified files in the process),
then start making some follow-up edits to sources while I am waiting
for Maven to finish because I realize I forgot something? I am going
to get a conflict and a confusing dialog in my IDE asking me to decide
whether to save my actual edits, or discard and reload from disk.
Perhaps the reformatting during a default lifecycle phase could be
something that you (the developer working locally, not the plugin
maintainer!) could explicitly opt in to, for example with a local
profile in `~/.m2/settings.xml`. Then you are at least expecting this
to happen and are making corresponding accommodations in your
development workflow.
--
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-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr0JHcOprUBit1EH-xsYbgtRJbsk_W5kuWy%2BEphVnLPzPw%40mail.gmail.com.
* text=auto
*.java eol=lf
The auto format should occur before the compile stage, probably during process-sources
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAA0qCNwGXcwroH95QyCORW4cYXx8h0BsUiLO4NwAZtF2Vh2Siw%40mail.gmail.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?