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

Cleanup Documentation #176

Merged
merged 18 commits into from
Apr 1, 2019
Merged

Cleanup Documentation #176

merged 18 commits into from
Apr 1, 2019

Conversation

jpb
Copy link
Contributor

@jpb jpb commented Mar 20, 2019

No description provided.

@tavisrudd
Copy link
Collaborator

Good start

@jpb jpb marked this pull request as ready for review March 26, 2019 19:42
@jpb jpb requested review from tavisrudd and tuff March 26, 2019 19:43
@jpb
Copy link
Contributor Author

jpb commented Mar 26, 2019

A more readable link for the README: https://github.com/unbounce/iidy/blob/cleanup-docs/README.md

docs/using-parameter-store.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
docs/yaml-preprocessing.md Outdated Show resolved Hide resolved
Copy link
Contributor

@tuff tuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few minor things but otherwise this looks great and really improves the ✨devX✨ of this tool.

README.md Outdated
* It provides immediate, readable feedback about what CloudFormation is doing
and any errors it encounters.
* It is simple to learn, understand, and use.
parts, that map directly to CloudFormation's
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some words are missing: "There are fewer moving parts and its commands map directly to CloudFormation's"

README.md Outdated
stack.
* It does some template validation, guards against common issues, and will be
extended over time to validate our best practices and security policies.
* It provides seemless integration with AWS Parameter Store
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be consistent with the final period on the items in this list.

README.md Outdated
## Usage

### Help
```

```shell
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should skip the shell here as the syntax highlighting is incorrect in the output.

README.md Outdated
@@ -94,98 +93,116 @@ Commands:
iidy update-stack <argsfile> update a cfn stack based on stack-args.yaml
iidy create-or-update <argsfile> create or update a cfn stack based on stack-args.yaml
iidy estimate-cost <argsfile> estimate aws costs based on stack-args.yaml

...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historically I've been replacing the ...s here with blank lines.

README.md Outdated

Status Codes:
Success (0) Command successfully completed
Error (1) An error was encountered while executing command
Cancelled (130) User responded 'No' to iidy prompt or interrupt (CTRL-C) was received
```

### The `argsfile` (aka `stack-args.yaml`)
### Environment Variables
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should discourage people from wrapping iidy in complex shell scripts that use these envvars. For that reason, I'd move this section done.

README.md Outdated
# example stack-args.yaml
#### Importing Data

Data can be imported into the stack args file using the `$imports` block.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$imports: with the colon

Type: AWS::SNS::Topic
Properties:
DisplayName: "Hello {{Bar}}"
- CLI options (see above)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a better spot to cover the envvars than above.

# Converting Existing CloudFormation Stacks to iidy

Converting current stacks to iidy can be done through the iidy
convert-stack-to-iidy` command, which accepts two parameters: the stack name of
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an opening backtick was missed here.

stored. For example:

```shell
iidy convert-stack-to-iidy my-cloudformation-stack-1 .
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the example, I think we should use a sub-dir.

@@ -0,0 +1,36 @@
### Using Parameter Store for Secrets
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also cover the iidy params commands here.

README.md Outdated

Any parameter used by iidy can be set using IIDY_{{argname}}
## Additional Documentation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should lead with the readme with TOC of all documentation for easy reference.

# - c
```

* `!$escape` 🤷‍️
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some-entry: !$escape "{{ this is escaped }}"
some-entry: "{{ this is escaped }}"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry2: $escape
  - $! testing
entry2:
  - $! testing

# - c
```

* `!$escape` 🤷‍️
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.e. $!escape prevents `iidy from doing any pre-processing on the child tree.

@tavisrudd
Copy link
Collaborator

I think it would be helpful to provide some examples of the --query / jmespath stuff for getting values out of stacks or the environment as part of shell scripts that wrap iidy.

@jpb jpb requested a review from tavisrudd March 28, 2019 17:51
@jpb jpb merged commit ea33735 into master Apr 1, 2019
| `git:describe` | Get current git description (via `git describe --dirty --tags`) | `git:describe` |
| `git:sha` | Get current git sha name (via `git rev-parse HEAD`) | `git:sha` |
| `random:dashed-name` | Generate a random dashed name, for example: `'uptight-guitar'` | `random:dashed-name` |
| `random:name` | Generate a random dashed name, for example: `'uptightguitar'` | `random:name` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description for random:name seems to have been copied from name:dashed-name but not changed.

@tavisrudd tavisrudd deleted the cleanup-docs branch April 26, 2019 17:45
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.

4 participants