-
Notifications
You must be signed in to change notification settings - Fork 25
Implement update commands for commonly-used entities
#345
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
Conversation
|
|
||
| // ExpressionIndexes with mapped ids or identical expressions are assumed to be equivalent. | ||
| var immutableTarget = template.ResourceGroup != "ExpressionIndexes"; | ||
| var immutableTarget = template.ResourceGroup == "ExpressionIndexes"; |
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.
Unfortunately, this bug breaks template import in the latest version
| { | ||
| static int ServerListenPort => 9989; | ||
| static int _nextServerPort = 9989; | ||
| readonly int _serverListenPort = Interlocked.Increment(ref _nextServerPort); |
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.
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.
|
|
||
| // 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); |
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.
have you flipped the bool here?
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.
Thanks; yes, that one's the fix 👍
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.
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.
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.