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

Added infrastructure to safely bump flake8 version #333

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

elautz
Copy link
Contributor

@elautz elautz commented Dec 8, 2019

Added logic in the Flake8Task to rerun the execution with ignoring
a set of rules if the first execution fails. It will rerun if the
IGNORE_RULES set it non-empty. This set of rules will contain
any rules/checks added between flake8 versions. For example, when
we bump to flake8 3.8.0, we should update the IGNORE_RULES value
to hold all rules and checks added between flake8 3.6.0 and 3.8.0.

If the second run, when ignoring newly added rules, succeeds, we
now emit a warning to the user stating which rules are being
ignored, and which of these ignored rules failed for them. Added a
method called overrideIgnoreRules() which is used for integration
testings and will be used internally by LI. Integration tests
were added to verify this new functionality.

@elautz elautz requested a review from zvezdan December 8, 2019 03:49
Copy link
Member

@zvezdan zvezdan left a comment

Choose a reason for hiding this comment

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

Excellent!
Just small nits.

Added logic in the Flake8Task to rerun the execution with ignoring
a set of rules if the first execution fails. It will rerun if the
IGNORE_RULES set it non-empty. This set of rules will contain
any rules/checks added between flake8 versions. For example, when
we bump to flake8 3.8.0, we should update the IGNORE_RULES value
to hold all rules and checks added between flake8 3.6.0 and 3.8.0.

If the second run, when ignoring newly added rules, succeeds, we
now emit a warning to the user stating which rules are being
ignored, and which of these ignored rules failed for them. Added a
method called overrideIgnoreRules() which is used for integration
testings and will be used internally by LI. Integration tests
were added to verify this new functionality.
@elautz elautz merged commit 33f2c7d into linkedin:master Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants