See if run changes will work #1194
Closed
Conversation
This file contains 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
The fact that using a transaction object to do a commit results in a non-transaction should be documented so that if we decide to introduce a change later where this behaves differently then it is well documented. # Conflicts: # test/transaction.ts
When begin transaction sends back an error, we want some tests to capture what the behavior is so that when we make changes to the run function then behavior is preserved.
A response should reach the user the right way. Add tests to make sure behavior is preserved.
In the run function delegate calls to runAsync and use run async to make promise calls
This allows this function to return a promise instead of a promise wrapped in a promise. This makes the tests pass and behave the way they should.
Do not call request from self
Comments should actually explain what is being done
The commit test for this PR should be removed because it is not really relevant for the async run functionality.
The types used should be very specific so that reading the code isn’t confusing.
Add a type to the resolve function just to introduce more clarity
product-auto-label
bot
added
size: m
Make the types more specific in the data client callback so that it is easier to track down the signature and match against the begin transaction function.
The parsing logic is going to be needed elsewhere so taking it apart now.
The interface name should be changed so that it matches what it is. It is the callback used to form a promise.
Change accessors to hide data completely instead of using the private modifier
Eliminate the early return as suggested in the PR
The comments capture the parameters and return type.
runAsync should be in promisfy excludes
…c functions and other read/write calls
…nieljbruce/nodejs-datastore into nodejs-transaction-change-run
Make sure it is explicit that we are parsing begin results.
Modify comment so that it doesn’t reference the way the code was before.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Draft PR: Testing if changes to run will break integration tests