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

Count only the parameters in the parameter set #1608

Merged
merged 3 commits into from Jun 2, 2021

Conversation

divbhasin
Copy link
Contributor

@divbhasin divbhasin commented May 29, 2021

What does this change

When using porter parameters generate..., we see only the number of parameters in the parameter set being generated now. Look at the "Porter Command and Output" screenshot in #1600 to see what was wrong before. For the same command, we now get:

Screen Shot 2021-05-29 at 9 44 49 AM

What issue does it fix

Closes #1600

Notes for the reviewer

For the "1 parameters declared for bundle..." in the above screenshot, should I change the "parameters" to "parameter(s)" to account for the above case where we might only have 1 parameter in the parameter set? Alternatively, we could also do a switch/if statement to be more precise.

Checklist

  • Unit Tests
  • Documentation
  • Schema (porter.yaml)

Signed-off-by: div_bhasin <divbest99@gmail.com>
Signed-off-by: div_bhasin <divbest99@gmail.com>
@@ -25,8 +25,9 @@ credentials:
- uninstall

dependencies:
- name: mysql
reference: "getporter/azure-mysql:5.7"
requires:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is unrelated to the PR. Without it, the "TestManifestConverter_generatedMaintainers" was failing.

Signed-off-by: div_bhasin <divbest99@gmail.com>
Copy link
Member

@carolynvs carolynvs 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 fixing this, and also the bonus patch for our main build being broken! 💯

@carolynvs carolynvs merged commit fea7bb9 into getporter:v1 Jun 2, 2021
@carolynvs
Copy link
Member

Sorry I forgot to respond to your question about "parameter(s)". Yes, if you would like to submit a PR to change it to "parameter(s)" that would be great.

Also congrats on your first pull request to Porter! 🎉 We like to thank our contributors by having them add their name to our contributors.md file at the root of the repository. You are welcome to add your name to the file anytime.

@divbhasin divbhasin mentioned this pull request Jun 6, 2021
3 tasks
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.

None yet

2 participants