Skip to content

Comments

feat: Backwards compatibility for flags#2510

Closed
bitdivine wants to merge 4 commits intomasterfrom
backwards-compatible-flags
Closed

feat: Backwards compatibility for flags#2510
bitdivine wants to merge 4 commits intomasterfrom
backwards-compatible-flags

Conversation

@bitdivine
Copy link
Contributor

@bitdivine bitdivine commented Aug 29, 2022

Description

The location of some flags has moved. For example:

Old: dfx canister --network local id mydapp
New: dfx canister id --network local mydapp

This is quite disruptive, as it breaks almost every script that uses dfx.

This PR implements backwards-compatibility by accepting the old format in a few common cases. It does not attempt to cover all possible situations where we know how to rewrite from the old syntax to the new. It removes 90% of the pain with 10% of the code.

Alternatives considered

Update all the code. This would however break any case where say sns was used with an older version of dfx. Also, updating to this latest dfx would break our CI in many places and I doubt we could fix it all at once.

How Has This Been Tested?

I have used this for an SNS deployment, as that uses quite a few scripts in our inventory, as well as the sns binary which has calls to dfx baked into the code.

If this general approach seems OK I am happy to write bash tests as well.

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@bitdivine bitdivine force-pushed the backwards-compatible-flags branch from cfb8b8f to 7957415 Compare August 29, 2022 14:10
@bitdivine bitdivine marked this pull request as ready for review August 29, 2022 14:10
@bitdivine bitdivine requested a review from a team as a code owner August 29, 2022 14:10
@ghost
Copy link

ghost commented Aug 31, 2022

Closing because #2521 has made the changes in a backwards-compatible way.

@ghost ghost closed this Aug 31, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant