Skip to content

Conversation

RandyMcMillan
Copy link
Contributor

On macOS the default sed utility is not GNU sed,
therefore the commit-script-check.sh script fails.
This commit adds a check for gsed and uses it if it exists.

@DrahtBot DrahtBot added the Tests label Jan 26, 2022
Copy link
Contributor

@w0xlt w0xlt left a comment

Choose a reason for hiding this comment

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

crACK 0b4f7a9

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

Approach ACK 0b4f7a9

This PR introduces a check for the sed type in the commit-script-check.sh. The selected version of SED (whether gsed or sed) is then passed through the inspection if the sed is 'GNU' based.

I think it would be good incorporating @brunoerg's suggestion.

@RandyMcMillan
Copy link
Contributor Author

merged @brunoerg commit - will squash after CI pass.

On macOS the default sed utility is not GNU sed,
therefore the commit-script-check.sh script fails.
This commit adds a check for gsed and uses it if it exists.

Co-authored-by: Bruno Garcia <brunoely.gc@gmail.com>
@RandyMcMillan RandyMcMillan force-pushed the 1643135199-test-lint-comit-script branch from c56df55 to ca3d686 Compare January 27, 2022 16:55
@theStack
Copy link
Contributor

Concept ACK

@laanwj
Copy link
Member

laanwj commented Mar 30, 2022

I know I'm repeating myself but I really prefer more complex scripts to be Python to avoid these kind of platform-specific contortions around different shells and basic utilities.

Lacking that, Concept ACK.

@fanquake
Copy link
Member

fanquake commented Apr 7, 2022

Going to optimistically close this for now, given we are just going to port this to Python (#24783). I'm sure we can come up with a solution that doesn't require sed, or gsed or bash, or any other nonsense.

@fanquake fanquake closed this Apr 7, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Apr 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants