Implement update commands for commonly-used entities#345
Merged
nblumhardt merged 3 commits intodatalust:devfrom May 3, 2024
Merged
Implement update commands for commonly-used entities#345nblumhardt merged 3 commits intodatalust:devfrom
update commands for commonly-used entities#345nblumhardt merged 3 commits intodatalust:devfrom
Conversation
nblumhardt
commented
May 3, 2024
|
|
||
| // ExpressionIndexes with mapped ids or identical expressions are assumed to be equivalent. | ||
| var immutableTarget = template.ResourceGroup != "ExpressionIndexes"; | ||
| var immutableTarget = template.ResourceGroup == "ExpressionIndexes"; |
Member
Author
There was a problem hiding this comment.
Unfortunately, this bug breaks template import in the latest version
nblumhardt
commented
May 3, 2024
| { | ||
| static int ServerListenPort => 9989; | ||
| static int _nextServerPort = 9989; | ||
| readonly int _serverListenPort = Interlocked.Increment(ref _nextServerPort); |
Member
Author
There was a problem hiding this comment.
These changes are to support running each test case against a new, unique server instance. Previously, a failure in one test could leave the server in an unsuitable state for later tests, making it difficult to see which test actually failed.
liammclennan
reviewed
May 3, 2024
|
|
||
| // ExpressionIndexes with mapped ids or identical expressions are assumed to be equivalent. | ||
| var immutableTarget = template.ResourceGroup != "ExpressionIndexes"; | ||
| var immutableTarget = template.ResourceGroup.Equals("ExpressionIndexes", StringComparison.OrdinalIgnoreCase); |
Contributor
There was a problem hiding this comment.
have you flipped the bool here?
Member
Author
There was a problem hiding this comment.
Thanks; yes, that one's the fix 👍
Member
Author
There was a problem hiding this comment.
Previously the condition "worked" because it failed to take different casings of "ExpressionIndexes" into account, and so happened to return the correct value for that case.
liammclennan
approved these changes
May 3, 2024
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
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.
Also fixes #346.
Automating with a shell and
seqcliis great, except when an existing entity needs to be edited. So far no commands support any kind ofupdateoredit, because there's so much variation between entities and complexity in specifying properties.This PR proposes an immediate blanket solution, using the JSON format to round-trip edits.
This works well with modern shells, e.g. PowerShell (edited for clarity):
Future type-specific extensions to
updatemight provide alternative ways of setting properties.