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

Update Example of Parsing a Manifest to Use Only jq #12894

Merged
merged 2 commits into from
May 15, 2024

Conversation

estheruary
Copy link
Contributor

Documentation change, the example provided uses a command like jq ... | cut -f: -d2 where jq can perform the splitting itself, no need to pipe.

Little one-line change, noticed it today while looking to parse the manifest format.

@estheruary estheruary requested a review from a team as a code owner March 20, 2024 14:29
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 20, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Hi @estheruary,

Thanks for the fix! It does seem to work with this approach here, I don't mind a one command solution for this example.

I left a comment regarding the split though, not sure if the space was intentional or not, I'll let you address that and I'll do another pass.

Also I notice you haven't signed our CLAs, which will not allow us to merge your PR unfortunately for legal reasons, would it be possible for you to sign it?

We'll be able to merge that once both these items are addressed, thanks!

@@ -201,7 +201,7 @@ build.

#!/bin/bash

AMI_ID=$(jq -r '.builds[-1].artifact_id' manifest.json | cut -d ":" -f2)
AMI_ID=$(jq -r '.builds[-1].artifact_id | split(": ") | .[1]' manifest.json)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the space after the : is a typo? The resulting artifact ID should be in the <region>:<id> format, without space, unless I'm missing something, please correct me if so.

@nywilken
Copy link
Contributor

Hi @estheruary thanks for making this fix. Please let us know if you wish to continue by signing the CLA.

@estheruary
Copy link
Contributor Author

@nywilken Done!

@nywilken nywilken added backport/website Backport PR changes to `stable-website` and latest release branch and removed needs-cla labels May 15, 2024
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

Thank you @estheruary I will merge once all goes green again. Thank you for following up and signing the CLA.

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2024
@estheruary estheruary deleted the patch-1 branch October 5, 2024 03:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/website Backport PR changes to `stable-website` and latest release branch docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants