Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify to also validate formatting #82

Closed
djarnis73 opened this issue Dec 11, 2020 · 11 comments
Closed

Verify to also validate formatting #82

djarnis73 opened this issue Dec 11, 2020 · 11 comments
Assignees

Comments

@djarnis73
Copy link

Hi

When running in either a git commit hook or on a build server it would make sense if verify could also validate formatting (indentation etc).

Would that be difficult to implement?

In the project I'm most involved with, it would reduce merging conflicts if the build would fail if people did not apply correct formatting.

Best regards Jens

@Ekryd
Copy link
Owner

Ekryd commented Dec 12, 2020

Hi,

So you are looking for some kind of 'strict' formatting for the verify goal, i.e. that same checks that the ordinary sort goal does?

I don't think it is too hard to implement.

Been thinking about parameter name and the values that parameter can have:
verifyDetails: 'xmlElements' (default) or 'strict'

An alternative to 'verifyDetails' are 'verifyChecking' or ''verifyLevel. Also do you have an alternative to the value 'strict'?

Best regards, Björn

@djarnis73
Copy link
Author

Yes, I think I would be happy with a flag that would let sortpom:verify fail if the pom is not formatted like sortpom:sort, but there could of course be levels of strictness to verify.

We are currently using the tidy plugin, but I'm getting increasingly annoyed at whitespace related merge conflicts, which I think could be reduced if the build would fail if both sorting and formatting was checked.

Parameter could also be named: failOn and the value could be a set/list of thing to check/verify.

Best regards Jens

@Ekryd Ekryd self-assigned this Dec 27, 2020
@create-issue-branch
Copy link

Branch issue-82-Verify_to_also_validate_formatting created!

@Ekryd
Copy link
Owner

Ekryd commented Dec 29, 2020

What do you think about this:

    /**
     * What kind of differences should trigger verify failure. Can be either 'xmlElements', 'lines' or 'strict'
     */
    @Parameter(property = "sort.verifyFailOn", defaultValue = "xmlElements")
    private String verifyFailOn;

xmlElements is the default value where only the order of the xml elements is considered
lines is that each line in the file must match
strict is that each line and line endings must match

Ekryd pushed a commit that referenced this issue Jan 2, 2021
Ekryd pushed a commit that referenced this issue Jan 2, 2021
Ekryd pushed a commit that referenced this issue Jan 2, 2021
Ekryd pushed a commit that referenced this issue Jan 4, 2021
…PomImpl and SortPomService. Fixed coverage
@Ekryd
Copy link
Owner

Ekryd commented Jan 10, 2021

Time for testing!

  • Clone the repository

  • Checkout the master branch

  • Build with mvn clean install

  • Use the plugin with mvn com.github.ekryd.sortpom:sortpom-maven-plugin:2.12.1-SNAPSHOT:verify and remember to use the new parameter -Dsort.verifyFailOn=strict or xmlElements. The parameter can also be combined with -Dsort.ignoreLineSeparators=false

  • Feel free to also use it by plugin configuration

Let me know how it works for you

@djarnis73
Copy link
Author

djarnis73 commented Jan 11, 2021

Hi Björn

Just did a quick test on my work project, when running like:

mvn com.github.ekryd.sortpom:sortpom-maven-plugin:2.12.1-SNAPSHOT:verify -Dsort.verifyFailOn=strict

It does not fail but instead it corrects the invalid pom.

Output (slightly modified for privacy reasons).

[INFO] Scanning for projects...
[INFO]
[INFO] -----------------------< group:test-project >-----------------------
[INFO] Building test-project 0.0.1
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- sortpom-maven-plugin:2.12.1-SNAPSHOT:verify (default-cli) @ test-project ---
[INFO] Verifying file test-project/pom.xml
[INFO] The line 4 is not considered sorted, should be '  <parent>'
[INFO] The file test-project/pom.xml is not sorted
[INFO] Sorting file test-project/pom.xml
[INFO] Saved backup of test-project/pom.xml to pom.xml.bak
[INFO] Saved sorted pom file to test-project/pom.xml
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

I expected it to fail the build without modifying the pom.xml.

Best regards Jens

@Ekryd
Copy link
Owner

Ekryd commented Jan 11, 2021

In that case you need to add -Dsort.verifyFail=stop if you want a failed verify to stop the build. Try it and see how it works.

@djarnis73
Copy link
Author

djarnis73 commented Jan 11, 2021

Ahh, sorry, my bad. I must admit that I find it a bit counter intuitive that the verify goal modifies the pom file by default, but then again, it is pretty well documented so maybe it is just me that is used to how tidy-maven-plugin works.

I tried it with -Dsort.verifyFail=stop and then it works as expected. Good work 👍 .

Best regards Jens

@Ekryd
Copy link
Owner

Ekryd commented Jan 13, 2021

Great to hear! I have to fix some things before releasing an official new version. Should be done by the weekend.

@Ekryd
Copy link
Owner

Ekryd commented Jan 16, 2021

Hi,

New official release 2.13.1 has the verifyFailOn option. Enjoy!

@Ekryd
Copy link
Owner

Ekryd commented Jan 25, 2021

Updated the documentation for the plugin. https://github.com/Ekryd/sortpom/wiki/Parameters and https://github.com/Ekryd/sortpom/wiki/VerifyFailOn

Have you tried the official version yet?

@Ekryd Ekryd closed this as completed Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants