Skip to content

config: update 'verify-no-exception-configs' validation to not fail build if some contribution PR is referenced in description#7296

Merged
romani merged 1 commit into
checkstyle:masterfrom
romani:new-check-contribution
Dec 4, 2019
Merged

config: update 'verify-no-exception-configs' validation to not fail build if some contribution PR is referenced in description#7296
romani merged 1 commit into
checkstyle:masterfrom
romani:new-check-contribution

Conversation

@romani

@romani romani commented Dec 2, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

I really tiered to recheck Travis state when new Check is introduced ....
so here is extra hacking to ease this problem.
I do check any PR link as if it present it will be verified by human, so any link to contributions pools is ok for now. We can make validation more exact later on if required.

tested on #7228

testing:

diff --git a/config/checkstyle_checks.xml b/config/checkstyle_checks.xml
index dac232b..3cf89b3 100644
--- a/config/checkstyle_checks.xml
+++ b/config/checkstyle_checks.xml
@@ -77,6 +77,7 @@
   <module name="Translation">
     <property name="requiredTranslations" value="de, fr, fi, es, pt, ja, tr, zh"/>
   </module>
+  <module name="UniqueProperties111"/>
   <module name="UniqueProperties"/>
   <module name="OrderedProperties" />

~/java/github/romani/checkstyle [new-check-contribution L|✚ 2] 
$ export TRAVIS_PULL_REQUEST="7229" && READ_ONLY_TOKEN=vaaaalue && ./.ci/travis/travis.sh verify-no-exception-configs
.ci-temp/checks-nonjavadoc-error.xml:4.63: failed to load external entity "https://checkstyle.org/dtds/configuration_1_3.dtd"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
                                                              ^
failed to load external entity "-n"
.ci-temp/checks-only-javadoc-error.xml:4.66: failed to load HTTP resource
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
                                                                 ^
config/checkstyle_checks.xml:4.63: failed to load external entity "https://checkstyle.org/dtds/configuration_1_3.dtd"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
                                                              ^
PR Description grepped:
You introduce new Check
--- .ci-temp/web.txt	2019-12-02 00:17:52.695775967 -0800
+++ .ci-temp/file.txt	2019-12-02 00:17:52.707776418 -0800
@@ -155,6 +155,7 @@
 TypeName
 UncommentedMain
 UniqueProperties
+UniqueProperties111
 UnnecessaryParentheses
 UnnecessarySemicolonAfterTypeMemberDeclaration
 UnnecessarySemicolonInEnumeration
Please create PR to repository https://github.com/checkstyle/contribution
and add your new Check 
   to file checkstyle-tester/checks-nonjavadoc-error.xml
or to file checkstyle-tester/checks-only-javadoc-error.xml
PR for contribution repositoty will be merged right after this PR.

✘-1 ~/java/github/romani/checkstyle [new-check-contribution L|✚ 2] 
00:17 $ export TRAVIS_PULL_REQUEST="7228" && READ_ONLY_TOKEN=vaaaalue && ./.ci/travis/travis.sh verify-no-exception-configs
.ci-temp/checks-nonjavadoc-error.xml:4.63: failed to load external entity "https://checkstyle.org/dtds/configuration_1_3.dtd"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
                                                              ^
failed to load external entity "-n"
.ci-temp/checks-only-javadoc-error.xml:4.66: failed to load HTTP resource
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
                                                                 ^
config/checkstyle_checks.xml:4.63: failed to load external entity "https://checkstyle.org/dtds/configuration_1_3.dtd"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
                                                              ^
PR Description grepped:"Fix for issue #6971.\r\n\r\nContribution PR: https://github.com/checkstyle/contribution/pull/417\r\n\r\nRegression report:\r\nhttps://sd1998.github.io/checkstyle-regression/Fix-69

$ READ_ONLY_TOKEN=valuue && ./.ci/travis/travis.sh verify-no-exception-configs
.ci-temp/checks-nonjavadoc-error.xml:4.63: failed to load external entity "https://checkstyle.org/dtds/configuration_1_3.dtd"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
                                                              ^
failed to load external entity "-n"
.ci-temp/checks-only-javadoc-error.xml:4.66: failed to load HTTP resource
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
                                                                 ^
config/checkstyle_checks.xml:4.63: failed to load external entity "https://checkstyle.org/dtds/configuration_1_3.dtd"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
                                                              ^
✔

@romani romani requested review from pbludov and rnveach December 2, 2019 08:25
rnveach
rnveach approved these changes Dec 3, 2019
View reviewed changes
Comment thread .ci/travis/travis.sh
if [[ $DIFF_TEXT != "" ]]; then
if [[ $TRAVIS_PULL_REQUEST =~ ^([0-9]+)$ ]]; then
LINK_PR=https://api.github.com/repos/checkstyle/checkstyle/pulls/$TRAVIS_PULL_REQUEST
REGEXP="https://github.com/checkstyle/contribution/pull/"

@rnveach rnveach Dec 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 things:

  1. It might be nice if there is a check that the PR in other repo actually fulfills missing Check(s).
  2. When merged and master CI runs, it would be nice if we check that the referenced PR is already merged, otherwise master should fail.

@romani romani Dec 3, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice if there is a check that the PR in other repo actually fulfills missing Check(s).

#7304

it would be nice if we check that the referenced PR is already merged

as this validation is executed on master too, it will fail.
As it looks for PR only when there is a diff between files in master of both repositories.

Comment thread .ci/travis/travis.sh Outdated
@rnveach rnveach assigned romani and unassigned rnveach Dec 3, 2019
rnveach
rnveach reviewed Dec 3, 2019
View reviewed changes

@rnveach rnveach left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just thought of this.

Comment thread .ci/travis/travis.sh
Comment thread .ci/travis/travis.sh
@romani romani mentioned this pull request Dec 3, 2019
Closed
@romani
config: update 'verify-no-exception-configs' validation to not fail b…
115cfdd 
…uild if some contribution PR is referenced in description
@romani romani merged commit 5737732 into checkstyle:master Dec 4, 2019
@romani romani deleted the new-check-contribution branch December 4, 2019 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@rnveach rnveach rnveach approved these changes

@pbludov pbludov pbludov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@romani romani

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@romani @pbludov @rnveach