-
Notifications
You must be signed in to change notification settings - Fork 816
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
fix: Enable, instead of Configure, conflict detection #6708
fix: Enable, instead of Configure, conflict detection #6708
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6708 +/- ##
=======================================
Coverage 56.80% 56.80%
=======================================
Files 486 486
Lines 22030 22030
Branches 4400 4400
=======================================
Hits 12515 12515
Misses 8638 8638
Partials 877 877 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, contingent upon verifying there aren't any scripts or ancillary processes that rely on this wording.
Do we need to make doc updates as well? I believe our DataStore docs have references to conflict detection in the CLI.
Yes, it would be great if someone from the CLI team can confirm this change. I just did a simple find and replace, and am not familiar with the codebase at all.
Good call! Here's a PR to update the docs: aws-amplify/docs#2981 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @renebrandel confirmed that this changed was approved in an API review
Thanks @jhockett! Please go ahead and merge this if you can, as I don't have write permissions to do so. |
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Customers often struggle quite a bit to figure out how to enable/disable conflict detection with the CLI. They can do it via the top level "Enable DataStore for entire API" or "Disable DataStore for entire API" options, but if they choose to walk through all options, they are forced to explicit decide if conflict detection should be enabled or not. I've been confused by this myself. Initially I thought that the "Configure conflict detection?" step was just a question asking if I wanted to make a change to the current value. That's not the case. If you answer "y", it enables conflict detection. If you answer "n", it disables conflict detection. This PR changes the wording so that customers know that this is causing an action.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.