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

Escape backslashes #144

Merged
merged 2 commits into from
Jun 10, 2019
Merged

Conversation

clalancette
Copy link
Contributor

@clalancette clalancette commented Jun 7, 2019

When running on Windows Debug (where all warnings are printed), the backslashes in these strings/comments were causing warnings like:

C:\J\workspace\nightly_win_deb\ws\install\Lib\site-packages\ament_cpplint\cpplint.py:1110: DeprecationWarning: invalid escape sequence \D

In cpplint.py, escape the backslashes properly (since these are triple-quoted strings). In cmakelint.py, make the strings raw.

@clalancette
Copy link
Contributor Author

Debug CI; these warnings no longer exist there: https://ci.ros2.org/job/ci_windows/7142/

@dirk-thomas
Copy link
Contributor

Both changed files are imported versions of upstream projects. Please file PRs against upstream first and once those have been accepted we can pull their latest version into this repo.

@clalancette
Copy link
Contributor Author

I've opened richq/cmake-lint#36 and google/styleguide#455

That being said, I won't hold my breath for either them to be merged. There have been no accepted changes to upstream cmake-lint since 2017, and the author has said he isn't really interested in it anymore: https://github.com/richq/cmake-lint/issues/29#issuecomment-431682169

The Google styleguide is slightly more active, but after 4 years they still haven't merged the python3 support patch that we are currently relying on: google/styleguide#47

@dirk-thomas
Copy link
Contributor

Please revert the unrelated whitespace changes from this PR. Then I think we can merge this without squashing. 👍

This gets rid of DeprecationWarning on Windows.

Signed-off-by: Chris Lalancette <[email protected]>
This fixes DeprecationWarning on Windows.

Signed-off-by: Chris Lalancette <[email protected]>
@clalancette clalancette force-pushed the clalancette/escape-cpplint-backslash branch from 3b2a1ec to ac2470d Compare June 7, 2019 14:28
@clalancette
Copy link
Contributor Author

Please revert the unrelated whitespace changes from this PR. Then I think we can merge this without squashing. +1

Rebased and done.

@clalancette clalancette added the in review Waiting for review (Kanban column) label Jun 7, 2019
@clalancette
Copy link
Contributor Author

CI was done as part of ros2/ci#301, and was successful, so I'll merge this.

@clalancette clalancette merged commit 45d8069 into master Jun 10, 2019
@clalancette clalancette deleted the clalancette/escape-cpplint-backslash branch June 10, 2019 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants