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

Fix linter errors from #131. #132

Merged
merged 1 commit into from
Aug 27, 2018
Merged

Fix linter errors from #131. #132

merged 1 commit into from
Aug 27, 2018

Conversation

nuclearsandwich
Copy link
Member

This fixes the linters which have been failing on nightlies over the weekend since #131.

Build Status

It may be that we don't care about these issues or have different preferences for resolving them in which case this PR may be amended to provide a different solution.

@wjwwood wjwwood merged commit b39645c into master Aug 27, 2018
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Aug 27, 2018
@wjwwood
Copy link
Member

wjwwood commented Aug 27, 2018

Thanks!

@nuclearsandwich nuclearsandwich deleted the linter-fixes branch August 27, 2018 17:19
@dirk-thomas
Copy link
Member

Can either of you reproduce the behavior of the buildfarm regarding the import order? Locally the previous state passed the linter and the new state doesn't. And from the changelog it looks like lexicographic ordering is expected (favoring the previous state).

@dirk-thomas
Copy link
Member

Nvm, after an update the config wasn't working anymore. It needs: ignore = I202 and import-order-style = google to reproduce the behavior in my editor (basically our custom configuration).

@nuclearsandwich
Copy link
Member Author

Yeah the sort ordering was what I was most suspicious about. I was able to reproduce both errors locally running the tests via colcon test. I usually end up copying or linking the config from ament_flake8 into ros2 projects I'm working on so that editor based feedback matches the buildfarm linter configuration.

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