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.

Sign up for GitHub

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

Jump to bottom

fix #4688 #4694

Merged
merged 1 commit into from
Dec 19, 2022
Merged

fix #4688 #4694

merged 1 commit into from
Dec 19, 2022

Conversation

matkoniecz
Copy link
Member

@matkoniecz matkoniecz commented Dec 17, 2022 •
edited
Loading

making it PR as I have not figured out what was being fixed in 1cc3c96 as far as user visible changes go

also, sadly I have not figured out how to add a nice test in this case that would test for this regression

fix #4688

@matkoniecz matkoniecz marked this pull request as ready for review December 17, 2022 20:30
@matkoniecz
Copy link
Member Author

Also, maybe filling newNames (and still not showing them to user) would be a better fix.

@westnordost
Copy link
Member

Thank you for the fix!

@westnordost westnordost merged commit 7a976ed into streetcomplete:master Dec 19, 2022
@matkoniecz
Copy link
Member Author

Do you remember what 1cc3c96 was fixing?

Was it supposed to have some user-visible effects or was it changing internal system to start behaving as advertised?

@matkoniecz matkoniecz deleted the nsi branch December 19, 2022 05:31
@westnordost
Copy link
Member

Yes,

  • noname and name:signed was not removed when a name was specified
  • other previously specified names were not removed when they were removed in the UI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NSI not adding names
2 participants
@matkoniecz @westnordost