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

Make CLI validation commands appear on the web #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zacikpa
Copy link
Contributor

@zacikpa zacikpa commented Oct 4, 2021

We dropped the commands when shifting to the new system but agreed it would be nice to have them back. Solves #105.

@mukrop mukrop linked an issue Oct 6, 2021 that may be closed by this pull request
@zacikpa
Copy link
Contributor Author

zacikpa commented Oct 6, 2021

Just to provide a bit of reasoning behind this solution. I introduced a script validation/certs/utils/cli-validation.py that simply copies the CLI validation data from the individual vconfig.yml files to a single file in the _data folder. When adding new certs, I find it more comfortable to just insert the CLI command into the local vconfig.yml file. If I had to add it directly to the _data folder each time, I would probably forget most of the time :)

Copy link
Member

@mukrop mukrop left a comment

Choose a reason for hiding this comment

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

Seems nice. Two minor points to improve:

  • The copy script uses command-line arguments. Please document these somewhere, at least briefly. My preference would be directly in the script (outputting short description when run with an inappropriate number of orguments). The arguments numberr check (I suggest to add) will prevent errors when make call to the script is done with nonexistent variabled (and thus only one or no argument to the script).
  • _data/certs.yml seems to me a weird name for aggregating the validation commands (as there are no certs in it and only borderline information about certs). What about _data/cli-validation.yml or something in this direction? (Sidenote: I do agree that having the validation commands in the vconfig.yml file is the right way to go.)

@mukrop
Copy link
Member

mukrop commented Oct 6, 2021

Ah, and I see conflicts by merging the lowercase names first. Is it simple to rebase?
If not, I could rebase the other way around (though it's not preferred as it messes with the history).

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.

Bring back the validation strings for CLIs
2 participants