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

update style to satisfy new flake8 plugins #84

Merged
merged 1 commit into from
Aug 30, 2017
Merged

Conversation

dirk-thomas
Copy link
Contributor

Connect to ros2/ci#98.

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Aug 29, 2017
@dirk-thomas dirk-thomas self-assigned this Aug 29, 2017
@@ -1,5 +1,5 @@
[flake8]
ignore = D100,D101,D102,D103,D104,D105,D203,D212,D404
ignore = C816,D100,D101,D102,D103,D104,D105,D203,D212,D404
Copy link
Contributor

Choose a reason for hiding this comment

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

is this ignored because it conflicts with another enforced rule (C815?) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, C816 is a suggestion specific to Python 3.6+. Since we are targeting only 3.5 we can't use the suggested style.

license = licenses[args.add_missing[1]]
add_missing_header(file_descriptors, name, license, args.verbose)
add_missing_header(
file_descriptors, name, licenses[args.add_missing[1]], args.verbose)
Copy link
Contributor

Choose a reason for hiding this comment

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

what was the error we're addressing here? if it's that we don't want 'wasteful' variables I don't think we should enforce that across the whole codebase as a general rule, because it's usually done for improved readability. but it may have been for another reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

license is a built-in in Python and should not be used as a variable name.

@dirk-thomas dirk-thomas force-pushed the flake8_plugins branch 2 times, most recently from eb7feb6 to 9652e49 Compare August 29, 2017 23:09
Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm

@dirk-thomas dirk-thomas merged commit ad12c71 into master Aug 30, 2017
@dirk-thomas dirk-thomas deleted the flake8_plugins branch August 30, 2017 21:57
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Aug 30, 2017
kenji-miyake pushed a commit to kenji-miyake/ament_lint that referenced this pull request Oct 20, 2021
update style to satisfy new flake8 plugins
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.

None yet

3 participants