Skip to content

Order the targets in available_software.py #447

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 26 commits into from
May 14, 2025
Merged

Order the targets in available_software.py #447

merged 26 commits into from
May 14, 2025

Conversation

laraPPr
Copy link
Collaborator

@laraPPr laraPPr commented May 2, 2025

Also needed to fix the tests to take the ordering into account

@laraPPr laraPPr mentioned this pull request May 2, 2025
Merged
1 task
bedroge
bedroge reviewed May 2, 2025
View reviewed changes
@laraPPr
Copy link
Collaborator Author

laraPPr commented May 2, 2025 •
edited
Loading

Tested new workflow here, laraPPr#1. I hope the workflow will be triggered for the update available scripts pr once this pr gets merged. I'm not sure why it is not triggering the workflow in this pr.

@laraPPr
Copy link
Collaborator Author

laraPPr commented May 2, 2025

And the tests of the available_software.py also do not seem to be running but I ran them locally.

@laraPPr
Copy link
Collaborator Author

laraPPr commented May 2, 2025

The test for the changed available_software.py script are ran see https://github.com/EESSI/docs/actions/runs/14799376763

bedroge
bedroge reviewed May 6, 2025
View reviewed changes
@laraPPr laraPPr requested a review from bedroge May 7, 2025 19:45
@Neves-P Neves-P self-requested a review May 14, 2025 08:02
Neves-P
Neves-P requested changes May 14, 2025
View reviewed changes
Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

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

@laraPPr, looks good! I just made a small suggestion on the docs of a function in the docstring. Feel free to reject if you prefer the info to stay as is (since you already have code comments) 👍

.github/workflows/test_overview_available_software.yml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This workflow didn't run (as expected, given run the conditions), but it did run in @laraPPr's branch and works well: https://github.com/laraPPr/EESSI_docs/actions/runs/14799375963, so it's safe to assume it will run well when the conditions are met in the target branch.

@laraPPr @Neves-P
expand docstring
bdfee65 
Co-authored-by: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com>
@laraPPr
Copy link
Collaborator Author

laraPPr commented May 14, 2025

The linter is failing on yo docstring

laraPPr
laraPPr commented May 14, 2025
View reviewed changes
@Neves-P @laraPPr
Make linter happy
e246c06 
Co-authored-by: Lara Ramona Peeters <49882639+laraPPr@users.noreply.github.com>
@Neves-P
Copy link
Member

Neves-P commented May 14, 2025

The linter is failing on yo docstring

Sorry about that! Pushed your suggestions now, hopefully it passes now 👍

Neves-P
Neves-P approved these changes May 14, 2025
View reviewed changes
Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

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

lgtm!

@Neves-P Neves-P merged commit ff131ff into EESSI:main May 14, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Neves-P Neves-P Neves-P approved these changes

@bedroge bedroge Awaiting requested review from bedroge

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@laraPPr @Neves-P @bedroge