Skip to content
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

Add backcompat args for argument renames #1599

Merged
merged 2 commits into from
Oct 28, 2015
Merged

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Oct 27, 2015

In boto/botocore#703, there were some correction made
to the CamelCase->snake_case conversions.

However, in the CLI, we still need to support these arguments.

This fix for this is to:

  • Add a copy of the correct arg into the arg table with the
    "wrong" name.
  • Mark the "wrong" name entry as undocumented.

There were no unit tests for this module, so I added a few.
I also added some functional tests for some (but not all)
the rename cases, just to ensure the end to end functionality
is working as we expect.

cc @kyleknap @mtdowling @rayluo @JordonPhillips

In boto/botocore#703, there were some correction made
to the CamelCase->snake_case conversions.

However, in the CLI, we still need to support these arguments.

This fix for this is to:

* Add a copy of the correct arg into the arg table with the
  "wrong" name.
* Mark the "wrong" name entry as undocumented.

There were no unit tests for this module, so I added a few.
I also added some functional tests for some (but not all)
the rename cases, just to ensure the end to end functionality
is working as we expect.
@jamesls jamesls added the pr:needs-review This PR needs a review from a Member. label Oct 27, 2015
@kyleknap
Copy link
Contributor

The change looks good. The only thing I am curious about is how did you audit the arguments that needed an alias? We do not want to miss any to avoid breaking changes.

@kyleknap
Copy link
Contributor

🚢 otherwise

@jamesls
Copy link
Member Author

jamesls commented Oct 28, 2015

I linked to the gist in the original pull request: https://gist.github.com/jamesls/0d1005415d30d2f9f44a

Ran it against develop, ran it against the PR branch. Looked at the diff.

I suppose if we wanted to be really sure I'd write the equivalent code that uses the arg tables to verify that we didn't miss anything because of customizations renaming args.

@kyleknap
Copy link
Contributor

Ok I think that is fine then. I do not think customization renamings is an issue because we chose what to rename it to as xform_name is really only used for model driven commands, which we do not control directly.

@jamesls jamesls merged commit a960a2d into aws:develop Oct 28, 2015
thoward-godaddy pushed a commit to thoward-godaddy/aws-cli that referenced this pull request Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:needs-review This PR needs a review from a Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants