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

docs: remove help note for tf-plan.json restriction [CC-793] #1834

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

teodora-sandu
Copy link
Contributor

What does this PR do?

This PR updates the snyk --help command to remove the note about the Terraform plan file name restriction.

Where should the reviewer start?

Have a look at the markdown document for any spelling mistakes.

How should this be manually tested?

Run npm run generate-help and have a look at the sections that changed.

Any background context you want to provide?

The code changes were delivered under #1822.

What are the relevant tickets?

https://snyksec.atlassian.net/browse/CC-793

Screenshots

Screenshot 2021-04-16 at 16 22 15

@teodora-sandu teodora-sandu marked this pull request as ready for review April 16, 2021 15:24
@teodora-sandu teodora-sandu requested a review from a team April 16, 2021 15:24
@teodora-sandu teodora-sandu self-assigned this Apr 16, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Apr 16, 2021

Warnings
⚠️ You've modified files in src/ directory, but haven't updated anything in test folder. Is there something that could be tested?
Messages
📖

This PR will not trigger a new version. It doesn't include any commit message with feat or fix.

Generated by 🚫 dangerJS against 0d729a4

@@ -61,6 +61,7 @@ function parseIacFileData(fileData: IacFileData): any[] {
let yamlDocuments;

try {
// the YAML library can parse both YAML and JSON content
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Two last points now I read this again.

  1. Worth adding here that the reason we use YAML is it means we don't have to disambiguate between, JSON, single YAML doc and multiple YAML doc as the parser does it for us?
  2. In the spirit of the names of the other functions and to be explicit perhaps this should be called parseYAMLOrJSONFileData() to be clear that this will not support a terraform file for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it a bit clearer now? I've tried to address both points, hopefully it makes sense 😃

This commit updates the snyk --help command to remove the note on the Terraform plan file name restriction [CC-793]
Copy link
Contributor

@aron aron left a comment

Choose a reason for hiding this comment

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

🙌 Looks amazing, thanks for iterating on the wording!

@teodora-sandu teodora-sandu merged commit 6a288eb into master Apr 16, 2021
@teodora-sandu teodora-sandu deleted the docs/tf-plan-help-note branch April 16, 2021 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants