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

Increment feature id in tile speed layer #6726

Merged
merged 3 commits into from
Mar 24, 2024

Conversation

datwelk
Copy link
Contributor

@datwelk datwelk commented Nov 3, 2023

The tile layer currently contains the same id value of "1" for all linestring features in the speeds layer. There's a comment in the codebase that says the id should be incremented for each feature, but this does not happen for the linestring features. It does happen for the turn restriction features a few lines down.

This PR addresses the issue where linestring feature ids are not incremented.

@mjjbell
Copy link
Member

mjjbell commented Mar 17, 2024

LGTM. Can you add a changelog entry?

@datwelk datwelk force-pushed the hotfix/tile-id-increment branch from 49f8f0d to d91af12 Compare March 17, 2024 14:30
@datwelk
Copy link
Contributor Author

datwelk commented Mar 17, 2024

@mjjbell Added changelog entry and rebased on master. Do I need to squash both commits into one or is it fine like this?

@mjjbell
Copy link
Member

mjjbell commented Mar 17, 2024

@mjjbell Added changelog entry and rebased on master. Do I need to squash both commits into one or is it fine like this?

Thanks, it's fine, they will be squashed on merge.

@mjjbell
Copy link
Member

mjjbell commented Mar 17, 2024

Looks like some node tests are failing, because the underlying PBF varints for the IDs now make it bigger than previously expected. Can you update?

exports.test_tile = {'at': [17059, 11948, 15], 'size': 156539};

@mjjbell mjjbell merged commit c28ba66 into Project-OSRM:master Mar 24, 2024
20 checks passed
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