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

correct comments for "compress edge if it crosses a traffic signal" #5384

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

wangyoucao577
Copy link
Contributor

@wangyoucao577 wangyoucao577 commented Mar 1, 2019

Issue

#5385

What issue is this PR targeting? If there is no issue that addresses the problem, please open a corresponding issue and link it here.

Please read our documentation on release and version management.
If your PR is still work in progress please attach the relevant label.

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

PR #4327 implemented functions, but the comments wasn't up to date. When I read codes here, it makes me confused. So I'm going to help to correct the comments.

@wangyoucao577
Copy link
Contributor Author

Hi, can anyone help to review this issue? It's just correct comments and has been waiting for about 2 months. Thanks!

@wangyoucao577
Copy link
Contributor Author

Hi, anyone can help to review this PR? Thanks!

@akashihi
Copy link
Contributor

Hi, anyone can help to review this PR? Thanks!

Could you please provide more context?

@wangyoucao577
Copy link
Contributor Author

@akashihi

Hi, anyone can help to review this PR? Thanks!

Could you please provide more context?

The comment tells that Do not compress edge if it crosses a traffic signal. However, when I read the code, I found that the logic under the comment do compressing edge that crosses a traffic signal. I have described the behavior here. After blame the changes, the author only comment the old comments in PR #4327 when made the changes, but didn't remove them or correct them. So I'd like to correct the comments to avoid confuse people.

@akashihi
Copy link
Contributor

@wangyoucao577 Thank you for clarification!

@wangyoucao577
Copy link
Contributor Author

So would you please help to merge the PR either? I can not merge it by myself. Thanks!

@akashihi akashihi merged commit df9fda1 into Project-OSRM:master Aug 18, 2020
@akashihi
Copy link
Contributor

So would you please help to merge the PR either? I can not merge it by myself. Thanks!

Sorry! Completely forgot about it :(

@wangyoucao577 wangyoucao577 deleted the clean-comments branch August 24, 2020 11:59
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