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

perf: Iterate over only kLandmark tagged values in AddLandmarks #4873

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

kinkard
Copy link
Contributor

@kinkard kinkard commented Sep 2, 2024

Issue

Minor perf fix that caught my eye during work with Tagged Values.

Best to review with "Hide whitespace" mark checked, as only for loop is actually changed.

Tasklist

  • Add tests
  • Add #fixes with the issue number that this PR addresses
  • Update the docs with any new request parameters or changes to behavior described
  • Update the changelog
  • If you made changes to the lua files, update the taginfo too.

Requirements / Relations

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

@kinkard kinkard force-pushed the perf-iterate-only-landmarks branch 2 times, most recently from 4c5945c to a908d78 Compare September 6, 2024 09:58
@kinkard kinkard force-pushed the perf-iterate-only-landmarks branch from a908d78 to b9e3184 Compare September 13, 2024 18:29
kevinkreiser
kevinkreiser previously approved these changes Sep 17, 2024
Copy link
Member

@kevinkreiser kevinkreiser left a comment

Choose a reason for hiding this comment

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

lgtm just update changelog to resolve conflic

@kinkard kinkard force-pushed the perf-iterate-only-landmarks branch from 6569101 to 94c108f Compare September 19, 2024 16:48
@kevinkreiser kevinkreiser merged commit 5f48bfa into valhalla:master Sep 23, 2024
7 checks passed
@kinkard kinkard deleted the perf-iterate-only-landmarks branch October 10, 2024 15:43
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