sevntu-checkstyle / sevntu.checkstyle

Additional Checkstyle checks, that could be added as extension to EclipseCS plugin and maven-checkstyle-plugin, Sonar checkstyle plugin, extension for CheckStyle IDEA plugin.
http://sevntu-checkstyle.github.io/sevntu.checkstyle/
190 stars 147 forks source link

Pull #990: Define variable for non-main-files-suppressions file #990

Closed stoyanK7 closed 1 year ago

stoyanK7 commented 1 year ago

From https://github.com/checkstyle/checkstyle/pull/12755#discussion_r1112055478

So before mergin this PR we need 1 PR for checkstyle repo to make it as variable and 1 PR for sevnu to define this variable.

Related to https://github.com/checkstyle/checkstyle/pull/12761

rnveach commented 1 year ago

@stoyanK7 Are you still working on the PR in main repo? I need to see it with this to verify the whole flow will work.

stoyanK7 commented 1 year ago

@rnveach I will come back to it later today. I suppose you want me to connect it to this PR and see green CI?

rnveach commented 1 year ago

If you can make main repo CI green and show proof it is invoking the new stuff, then that will be even better as it will show it actually works.

stoyanK7 commented 1 year ago

@rnveach To prove that this PR works, I think we actually need to change pom.xml here and that's it:

<checkstyle.nonMain.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle_non_main_files_checks.xml</checkstyle.nonMain.configLocation>

to

<checkstyle.nonMain.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/checkstyle_non_main_files_checks.xml</checkstyle.nonMain.configLocation>
 stoyan @  zenbook:  ~/open-source/sevntu.checkstyle/sevntu-checks                                                                                                   pull/990-variable [!]
$ git diff                                                                                                                                                                          6:57:22 pm
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: sevntu-checks/pom.xml
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ sevntu-checks/pom.xml:23 @
    <!-- verify time version -->
    <checkstyle.version>10.7.0</checkstyle.version>
    <checkstyle.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle_checks.xml</checkstyle.configLocation>
-   <checkstyle.nonMain.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle_non_main_files_checks.xml</checkstyle.nonMain.configLocation>
-   <checkstyle.non-main-files-suppressions.file>config/checkstyle_non_main_files_suppressions.xml</checkstyle.non-main-files-suppressions.file>
+   <checkstyle.nonMain.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/checkstyle_non_main_files_checks.xml</checkstyle.nonMain.configLocation>
    <checkstyle.header>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/java.header</checkstyle.header>
    <checkstyle.regexp.header>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/java_regexp.header</checkstyle.regexp.header>
    <antrun.plugin.version>3.1.0</antrun.plugin.version>
______________________________________________________________________________________________________________________________________________________________________________________________
 stoyan @  zenbook:  ~/open-source/sevntu.checkstyle/sevntu-checks                                                                                                   pull/990-variable [!]
$ mvn clean verify     
...
ERROR] Failed to execute goal org.apache.maven.plugins:maven-antrun-plugin:3.1.0:run (ant-phase-verify) on project sevntu-checks: An Ant BuildException has occured: The following error occurred while executing this line:
[ERROR] /home/stoyan/open-source/sevntu.checkstyle/sevntu-checks/config/ant-phase-verify.xml:54: Unable to create Root Module: config {https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/checkstyle_non_main_files_checks.xml}.
[ERROR] around Ant part ...<ant antfile="config/ant-phase-verify.xml" />... @ 10:50 in /home/stoyan/open-source/sevntu.checkstyle/sevntu-checks/target/antrun/build-main.xml: cannot initialize module SuppressionFilter - Unable to find: /home/stoyan/open-source/sevntu.checkstyle/sevntu-checks/${checkstyle.non-main-files-suppressions.file}

We already provide the variable in main repo: https://github.com/checkstyle/checkstyle/blob/d6d906980759598be045000fd32f3567d2b7c3d5/.ci/validation.sh#L697

That got merged yesterday: https://github.com/checkstyle/checkstyle/pull/12761

rnveach commented 1 year ago

https://github.com/checkstyle/checkstyle/commit/40f409271dd0f6e387bd0be14313589e52c14881 is what I was looking for. It was merged without my review so i didn't realize.

Truthfully, this PR should have been made first, and then a PR in main repo referencing this and adding extra commit to test and show it is hooking in. (https://github.com/checkstyle/contribution/pull/747 as an example)

romani commented 1 year ago

I am not sure what I did wrong. What blocks this PR to be merged?

stoyanK7 commented 1 year ago

I think nothing blocks it?

rnveach commented 1 year ago

@stoyanK7 It seems there is issues with the travis runs and it is actually failing. This PR just had a 10 minute time out run and it was not confirmed to be working. Another PR is failing.

https://app.travis-ci.com/github/sevntu-checkstyle/sevntu.checkstyle/jobs/597291051

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.2.1:check (default) on project sevntu-checkstyle-sonar-plugin: Failed during checkstyle execution: Unable to find configuration file at location: https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-10.8.0/config/checkstyle_checks.xml: Could not find resource 'https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-10.8.0/config/checkstyle_checks.xml'. -> [Help 1]

stoyanK7 commented 1 year ago

@rnveach On it. That was expected after the release happened.