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

adding "udmi validate" tool #561

Merged
merged 17 commits into from
Jan 27, 2023
Merged

adding "udmi validate" tool #561

merged 17 commits into from
Jan 27, 2023

Conversation

grafnu
Copy link
Collaborator

@grafnu grafnu commented Jan 13, 2023

Adds "udmi validate" tool (alpha), and some minor improvements to validator output for increased usability.

source $UDMI_ROOT/etc/shell_common.sh

source $UDMI_ROOT/etc/find_udmi_profile.sh
source $(realpath $(dirname $(readlink -f $0))/..)/etc/udmi_preamble.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Honestly all this source'ing makes me nervous. Is there some other way we can accomplish this?

If we just want to put configuration data into shell scripts that's one thing, and carefully pull the values out. But at the point that we are building multiple source (with sourceing them) shell scripts I think we might need a different approach.

WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know how else to do it other than with "source" -- if you have a specific suggestion on "some other way" that's more appropriate then I'm all ears... but -- if you want/need to do something like define "common functions" I'm not sure how else to do it...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think if you are sourcing shell scripts you need to move to a different language.

@@ -1,18 +1,12 @@
#!/bin/bash -e

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there some reason these aren't named same_as_the_others instead of using - ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was basing it off of the git extension format, e.g. the file "git-utility" would map to the command-line "git utility " -- as a way to provide transparent extensions. So, "udmi-validate" maps to "udmi validate". The snake_case ones are generally things then that should be run directly (bin/test_sequencer). So, that was the reason (consistency with git extension naming)...

Copy link
Collaborator

@noursaidi noursaidi left a comment

Choose a reason for hiding this comment

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

Needs a bug/task for getting this documented, but worked nicely in my testing

@grafnu grafnu merged commit 599acff into faucetsdn:master Jan 27, 2023
@grafnu
Copy link
Collaborator Author

grafnu commented Jan 27, 2023 via email

@johnrandolph
Copy link
Collaborator

johnrandolph commented Jan 27, 2023 via email

@grafnu
Copy link
Collaborator Author

grafnu commented Jan 27, 2023 via email

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

3 participants