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

Object history preservation improvements #8839

Merged
merged 8 commits into from
Dec 7, 2021

Conversation

tpetillon
Copy link
Contributor

This PR extends what has been done in #7795 and #8708. It improves object ID preservation in the following cases:

  • merging nodes,
  • merging polygons,
  • merging nodes into a way.

To help doing this, the PR adds some utility functions dealing with object IDs.

It also fixes the order of relation members in some cases when splitting ways. (Depending on which way was the longest after the split, the two ways resulting from the split action could be inserted in the wrong order in the relation.)

Tests are provided for these actions.

@1ec5 1ec5 linked an issue Dec 5, 2021 that may be closed by this pull request
@tyrasd tyrasd added the enhancement An enhancement to make an existing feature even better label Dec 6, 2021
Copy link
Member

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

Wow, what a great PR. Huge thanks! 🙇

My only question/remark would be if it was really necessary to introduce the mew method osmEntity.id.isNew(id) in addition to osmEntity.isNew(). I don't immediately see why there would be a need for both methods. Wouldn't it be sufficient for this PR to simply call isNew() on the respective entity objects directly?

@tyrasd tyrasd added the priority A top priority issue that has a big impact or matter most to new mappers label Dec 6, 2021
@tpetillon tpetillon force-pushed the history_preservation branch from bab4b76 to 3ff06f9 Compare December 6, 2021 20:29
@tpetillon
Copy link
Contributor Author

You’re right, it wasn’t needed! I didn’t realise that while cleaning the code. I have removed it, and rebased the PR.

@tyrasd tyrasd merged commit 7714d47 into openstreetmap:develop Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement to make an existing feature even better priority A top priority issue that has a big impact or matter most to new mappers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cutted way filled in route in oposite direction
2 participants