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] Linter for v2 migration #11147

Merged
merged 13 commits into from
Jun 13, 2022
Merged

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Jun 13, 2022

As we are moving closer to v2 we think a linter would be the perfect tool to help us during this process. Here I'm modifying existing linters and proposing a new path:

  • linters will be compulsory, the PR will fail (and won't be merged) if any linter fail
  • all the rules executed by the linter should be stored in CCI repository, we need this flexibility to add and remove these rules and checks following our own migration pace.
  • Before adding a new rule, we need to check the number of recipes that will fail and how hard it will be to fix them. We might decide to postpone the addition if too many recipes are affected or the work required to fix them is too much at the moment.

Note. I will be adding an isolated yaml linter in the following hours

@ericLemanissier
Copy link
Contributor

It's great. Don't you think we will have to add this https://github.com/conan-io/conan/blob/develop/conans/pylint_plugin.py#L10-L47 to the CCI linter anyway, in order to avoid "undefined variable" warning/errors ?

@jgsogo
Copy link
Contributor Author

jgsogo commented Jun 13, 2022

Sure, this is just the starting point: a very naïve linter that works for 100% of the recipes.

Next steps will be to evolve the linter and check ConanFile attributes, methods,... but it can be done in following PRs. As you can see, those following PRs will run against all recipes, so we can know before merging how many recipes are broken and decide.

@ericLemanissier
Copy link
Contributor

ok, I had missed the disable=all, so these warnings won't show-up anyway. sorry!

@ericLemanissier ericLemanissier mentioned this pull request Jun 13, 2022
4 tasks
@conan-center-bot conan-center-bot merged commit 3365a3a into conan-io:master Jun 13, 2022
@jgsogo jgsogo deleted the v2/linter branch June 13, 2022 13:07
hoxnox pushed a commit to hoxnox/conan-center-index that referenced this pull request Jun 30, 2022
* [linter] propose linter for v2 migration

* touch

* remove requirements file

* share requirements as string/env variable

* add new line

* cache off

* version range

* exact requirement

* pylint already contains astroid

* Add comment about test_package

* better naming

* remove and rename files
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.

5 participants