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

GPII-3434: Combine settings action on key in with "reset all" #708

Merged
merged 12 commits into from Nov 27, 2018

Conversation

Projects
None yet
3 participants
@cindyli
Copy link
Contributor

commented Nov 9, 2018

This pull request is dependent on the pull request for GPII-3435, which should be merged in first.

To make it easier to review, the diff between these pull requests is here.

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 9, 2018

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 9, 2018

@cindyli cindyli requested a review from amb26 Nov 12, 2018

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 19, 2018

@@ -219,7 +219,7 @@ gpii.settingsHandlers.changesToSettings = function (element) {
return element.type === "DELETE" ? undefined : element.path ? {
path: element.path,
value: element.value
} : element.value;
} : fluid.isPrimitive(element) ? element : element.value;

This comment has been minimized.

Copy link
@amb26

amb26 Nov 19, 2018

Member

This is a bit funny, what case does this end up being tripped in? In general this kind of "flexible schema" leads to issues and we should try to retain the invariant that the payload is an Object with the value in "value" if possible

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 22, 2018

@cindyli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 22, 2018

@amb26, this pull request is ready too.

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 23, 2018

@amb26

This comment has been minimized.

Copy link
Member

commented Nov 26, 2018

Hi @cindyli - could you regenerate this pull so it is easier to see what is changed just here? Thanks

@cindyli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2018

@amb26, regenerated!

"lifecycleManager.loginRequestHandler": {
record: "gpii.lifecycleManager.userLogonHandling.matchMakingStateChangeHandler",
target: "{that gpii.lifecycleManager.loginRequest}.options.gradeNames"
},
// Both "defaultLifecycleInstructions" and "defaultSnapshot" structures are generated for performing

This comment has been minimized.

Copy link
@amb26

amb26 Nov 26, 2018

Member

This should really use the multi-line comment format - it is what it was invented for!

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 26, 2018

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Nov 26, 2018

@amb26 amb26 merged commit 18e0fde into GPII:master Nov 27, 2018

1 check passed

default Build finished.
Details

@cindyli cindyli deleted the cindyli:GPII-3434 branch Nov 27, 2018

@cindyli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 28, 2018

Merged at a394132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.