Introduce splitting preferences in chucks#33
Draft
tordans wants to merge 2 commits intoosmlab:mainfrom
Draft
Conversation
Author
|
@k-yle ping on this topic to request guidance and opinion on if and how you thing this could be moved forward. Especially the 3 questions above. |
Ensure updatePreference does not leave conflicting single/split copies after writes, and make merged preference reads deterministic so split values consistently win when valid. Add regression tests for migration paths and refresh e2e snapshots to keep the suite green with current API responses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I looked into how to implement #32
The discussion at openstreetmap/openstreetmap-website#6399 (comment) does not sound like a large limit will happen or at least not any time soon. Based on this, having tooling that handles the splitting / chucking will be very usefull.
This PR implements what @k-yle suggested in #32 (comment) - the way I understood it.
I spend most of my time working on the examples to see they make sense to me and cover edge cases.
The code and tests are generated an not reviewed by me, yet. — This will have to follow.
I also did not test the actual feature, yet. I was looking into setting up a /demo app that could be part of this repo which allows to use (a few) of the examples on the staging/production OSM server.
The main ideal of the change here is, that they replace the get/update/deletePreferences helper with new ones which allow to handle one key-value or split the values. They also add optional parsing/validation.
This means however, that the old APIs would be deprecated or removed. They are deprecated in this PR… except for
getPreferenceswhich I overwrote for a different purpose: Do get a list of all preferences for this user. So the current state is not ideal. I will either have to remove the two left over deprecations or remove the new function or rename the new function…So what is this draft about ATM: