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

Include a script to obtain gazebo releases for a given library #1148

Merged
merged 7 commits into from
Jun 10, 2024

Conversation

j-rivero
Copy link
Contributor

A helper script to obtain the Gazebo collection/releases given a library and a major version. It uses the gz-collections.yaml as the database to search for the metadata.

Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Just a few minor issues. LGTM!

contains a given library and major version that are provided as input
parameters.

The output is provid
Copy link
Contributor

Choose a reason for hiding this comment

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

stray sentence. Remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ouch fixed 02f5612

Comment on lines 32 to 33
Nothing if not found.ed as a space separated list in a single line.
Nothing if not found.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a sentence is repeated here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed also here 02f5612

Comment on lines 23 to 35
if len(sys.argv) < 3:
print(f"Usage: {sys.argv[0]} <lib_name> <major_version> <collection-yaml-file>")
sys.exit(1)

lib_name = sys.argv[1]
version = sys.argv[2]
yaml_file = sys.argv[3]

with open(yaml_file, 'r') as file:
data = yaml.safe_load(file)

collection_names = find_collection(data, lib_name, get_major_version(version))
print(f"{' '.join(collection_names)}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider putting this in an if __main__ block so this module can be imported by other Python code in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented the main and other improvements 3c62cde



if len(sys.argv) < 3:
print(f"Usage: {sys.argv[0]} <lib_name> <major_version> <collection-yaml-file>")
Copy link
Contributor

Choose a reason for hiding this comment

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

I won't block on this, but my preference would be to put more of the usage info in here rather than README.md. Users are accustomed to running a script or running with -h to see usage information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is also a good point but given that this repo uses README descriptions extensively, I'm going to to keep it consistent and store the details in as they are in the file.

Signed-off-by: Jose Luis Rivero <[email protected]>
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero j-rivero merged commit 40d7c77 into master Jun 10, 2024
1 check passed
@j-rivero j-rivero deleted the jrivero/get_collections_scrpt branch June 10, 2024 13:28
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.

2 participants