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.

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

trigger pre-hook in run_shell_cmd as early as possible so dry-run + trace output is correct #4544

Merged
merged 3 commits into from
Jun 5, 2024
Merged

trigger pre-hook in run_shell_cmd as early as possible so dry-run + trace output is correct #4544

merged 3 commits into from
Jun 5, 2024

Conversation

Micket
Copy link
Contributor

@Micket Micket commented May 30, 2024

Fixes #4490

@Micket Micket added change EasyBuild-5.0 EasyBuild 5.0 labels May 30, 2024
@Micket Micket added this to the 5.0 milestone May 30, 2024
@Micket Micket changed the base branch from develop to 5.0.x May 30, 2024 09:40
@Micket
Copy link
Contributor Author

Micket commented May 30, 2024

I think it's right but it's untested, will do so later

@boegel
Copy link
Member

boegel commented Jun 5, 2024

@Micket We should try and reproduce the bug in a test for run_shell_cmd, which will also confirm that the proposed changes are correct (they look OK to me)

@boegel boegel added the EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 label Jun 5, 2024
@Micket Micket marked this pull request as ready for review June 5, 2024 17:32
@boegel boegel changed the title Move run_shell_cmd hooks to run earlier trigger pre-hook in run_shell_cmd as early as possible so dry-run + trace output is correct Jun 5, 2024
@boegel boegel added bug fix and removed change labels Jun 5, 2024
boegel
boegel approved these changes Jun 5, 2024
View reviewed changes
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit 4223ede into easybuilders:5.0.x Jun 5, 2024
35 checks passed
@Micket Micket deleted the earlierhooks branch June 7, 2024 10:14
@boegel boegel mentioned this pull request Aug 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boegel boegel boegel approved these changes

Assignees
No one assigned
Labels
bug fix EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 EasyBuild-5.0 EasyBuild 5.0
Projects
EasyBuild v5.0
Status: No status
Milestone
5.0
Development

Successfully merging this pull request may close these issues.

run_shell_cmd hooks should be applied earlier
2 participants
@Micket @boegel