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

linter workflow: fix linter change test on test_package #15500

Merged
merged 11 commits into from
Feb 1, 2023

Conversation

ericLemanissier
Copy link
Contributor

@ericLemanissier ericLemanissier commented Jan 27, 2023

it has been broken by #15005
the error was

line 10: syntax error near unexpected token `)'

broken runs:

Instead of taking the 250 first recipes in alphabetical order, use shuf to randomize the selection
Also, fix the grouping of error messages, and order by decreasing occurance


this should fix the error message counting
@ericLemanissier ericLemanissier marked this pull request as ready for review January 27, 2023 14:37
@ericLemanissier
Copy link
Contributor Author

ericLemanissier commented Jan 27, 2023

it's also now faster to run.
Compare the new run with the old run

danimtb
danimtb previously approved these changes Jan 27, 2023
take advantage of the faster execution time
SSE4
SSE4 previously approved these changes Jan 28, 2023
There is a permissions issue
@prince-chrismc prince-chrismc self-assigned this Jan 29, 2023
@ericLemanissier
Copy link
Contributor Author

For review, the last summary generated by this PR: https://github.com/conan-io/conan-center-index/actions/runs/4035760268#summary-10955058022
Sorry for dismissing your reviews @SSE4 @danimtb. I won't touch the PR if no review requests changes.

@SSE4
Copy link
Contributor

SSE4 commented Jan 30, 2023

in future, I would love to move script/logic outside of the YAML (as it's a format for data, not for code).
these jq language also isn't something common and well-known.
maybe extracting these blocks to some scripts instead of these one-liners would help.
but PR is overall and improvement, as it leaves less code.

@ericLemanissier
Copy link
Contributor Author

@danimtb @prince-chrismc What do you think of this one ?

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

Can't be worse then already broken :)

@conan-center-bot conan-center-bot merged commit 448983c into conan-io:master Feb 1, 2023
@ericLemanissier ericLemanissier deleted the patch-4 branch February 1, 2023 05:48
StellaSmith pushed a commit to StellaSmith/conan-center-index that referenced this pull request Feb 2, 2023
* linter workflow: randomize the test of linter changes

* do a single pylint call for all tests

this should fix the error message counting

* sort messages by order of occurance

* Update linter-conan-v2.yml

* test 500 recipes

take advantage of the faster execution time

* post result as message on PR

* fixup

* Update linter-conan-v2.yml

* Update linter-conan-v2.yml

* Update linter-conan-v2.yml

* Remove message posting

There is a permissions issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants