Skip to content

Conversation

@pracucci
Copy link
Collaborator

@pracucci pracucci commented Dec 20, 2021

What this PR does:
In the PR #506 we've imported the cortex-jsonnet. In this PR I'm importing its README + CI linter (used to test the command in the README). To improve the testing, I've moved the getting started to a bash script so that we can test its whole execution.

Which issue(s) this PR fixes:
N/A

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Marco Pracucci <[email protected]>
@pracucci pracucci marked this pull request as draft December 20, 2021 15:16
@pracucci pracucci requested a review from simonswine December 21, 2021 08:30
@pracucci pracucci marked this pull request as ready for review December 21, 2021 08:30
Signed-off-by: Marco Pracucci <[email protected]>
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Looks good, nice test of the script.

Copy link
Contributor

@simonswine simonswine 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 for bringing this over. LGTM

A couple of thoughts I had about updates that would make sense at this point. None of them are hard requirements for merge.

Signed-off-by: Marco Pracucci <[email protected]>
@pracucci
Copy link
Collaborator Author

Thanks @pstibrany and @simonswine for your feedback! I've addressed your comments.

Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for addressing the comments

@pracucci pracucci merged commit f893e19 into main Dec 21, 2021
@pracucci pracucci deleted the import-jsonnet-readme branch December 21, 2021 09:36
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.

3 participants