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

Rework of CLI flag remapping #21

Merged
merged 5 commits into from
Jan 16, 2019
Merged

Rework of CLI flag remapping #21

merged 5 commits into from
Jan 16, 2019

Conversation

dansimau
Copy link
Contributor

@dansimau dansimau commented Jan 14, 2019

This reworks the flags feature and refactors a lot of the code under the hood
to change the design and fix some issues.

  • Fix the --help issue.

    Previously, --help didn't work at all and resulted in an error: pflag: help requested (see --help prints "pflag: help requested" #1). This fixes that issue and also improves the printing of
    user flags in the --help text. User flags are now shown at the bottom,
    separate from builtin and global astro flags. E.g.:

    $ astro plan --help
    Generate execution plans for modules
    
    Usage:
      astro plan [flags] [-- [Terraform argument]...]
    
    Flags:
          --detach           disconnect remote state before planning
      -h, --help             help for plan
          --modules string   list of modules to plan
    
    Global Flags:
          --config string   config file
          --trace           trace output
      -v, --verbose         verbose output
    
    User flags:
          --env string      Environment to deploy
          --region string   Region to deploy
    $
    
  • Previously, Terraform variable names were remapped to a different flag name
    by specifying the flag: option in the variable config, e.g.:

    - name: app
      path: core/app
      variables:
      - name: aws_region
        flag: region
    

    This meant if you had many modules defined with the same variable, you had to
    type out the mapping correctly every time or else you would get an error.

    The mapping configuration has now moved to its own block in the project
    config, which now looks like:

    flags:
      aws_region:
        name: region
        description: Region to deploy
    

    Users can now also specify a description that will appear in the --help text.

    Note that the flags section is completely optional, and be default astro will
    just expose the full variable name as the CLI flag.

  • Refactoring

    The implementation of user/project flags did a lot of the error checking in
    the cli/astro package itself, however, we want to move in a direction where
    those checks occur in the core astro package. The CLI should be a thin
    wrapper around the core package.

    Because this results in a breaking change in the config, I'm going to bump
    this to 0.5.0.

* Fixes --help
* Makes the --help output clearer (separating user flags from builtin
  flags
* Removes a lot of the flag checking from the CLI package (we should
  prefer to do this in astro itself).
@dansimau dansimau requested a review from DragonZ January 14, 2019 11:07
@dansimau dansimau self-assigned this Jan 14, 2019
@dansimau
Copy link
Contributor Author

@DragonZ I'm working on the tests now, but please go ahead and do a first pass. Thank you 🙏

@dansimau dansimau merged commit eef5b8a into master Jan 16, 2019
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.

2 participants