Skip to content

Renumber node IDs in .osrm.maneuver_overrides#4907

Merged
oxidase merged 1 commit intomasterfrom
renumber-nodeids
Feb 22, 2018
Merged

Renumber node IDs in .osrm.maneuver_overrides#4907
oxidase merged 1 commit intomasterfrom
renumber-nodeids

Conversation

@oxidase
Copy link
Copy Markdown
Contributor

@oxidase oxidase commented Feb 22, 2018

Issue

Fixes #4906

/cc @chaupow @TheMarex

Tasklist

Requirements / Relations

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

@oxidase oxidase requested review from TheMarex and chaupow February 22, 2018 09:55
@oxidase oxidase added this to the 5.16.0 milestone Feb 22, 2018
@oxidase oxidase requested a review from danpat February 22, 2018 10:19
Copy link
Copy Markdown
Member

@TheMarex TheMarex left a comment

Choose a reason for hiding this comment

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

Thanks for running this our. Looks like you sneaked a refactor and a second bug fix into this. 💯 Looks good to me. 👍

extractor::serialization::write(
writer, storage_maneuver_overrides, maneuver_override_sequences);

files::writeManeuverOverrides(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the refactor!

io::FileReader maneuver_overrides_file(config.GetPath(".osrm.maneuver_overrides"),
io::FileReader::VerifyFingerprint);
const auto number_of_overrides = maneuver_overrides_file.ReadElementCount64();
const auto number_of_overrides =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is another bug fix: Without skipping the bytes of the vector this would read garbage data.

@oxidase oxidase merged commit 83588fd into master Feb 22, 2018
@oxidase oxidase deleted the renumber-nodeids branch February 22, 2018 10:42
@oxidase oxidase self-assigned this Feb 22, 2018
@mjjbell mjjbell mentioned this pull request Mar 17, 2024
6 tasks
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