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

Add support for linting on macOS and enable GH workflow for macOS. #383

Merged
merged 8 commits into from
May 3, 2024

Conversation

kirkrodrigues
Copy link
Member

Description

The lint tasks are currently failing on macOS because of some Linux-specific command syntax that we're using. This PR switches to Linux/macOS-agnostic command syntax and enables the clp-lint workflow for macOS (so we have automated testing for linting on macOS).

Itemized changes:

  • Replace sed --regexp-extended with sed -E.
  • Remove --perl-regexp from the grep call and use the equivalent grep-compatible syntax.
  • Make the node version selection Linux/macOS-agnostic.
  • Document the requirements for running the linting tools.

Validation performed

  • Validated task lint:check and task lint:fix pass locally on a Linux host.
  • Validated the clp-lint workflow succeeds.
  • Sanity-checked that task package still works.

@kirkrodrigues kirkrodrigues requested a review from wraymo May 3, 2024 19:21
Copy link
Contributor

@wraymo wraymo left a comment

Choose a reason for hiding this comment

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

The title is good to me.

--max-count 1
"node-v\\d+\\.\\d+\\.\\d+-linux-{{.NODEJS_ARCH}}"
"node-v[[:digit:]]\+\.[[:digit:]]\+\.[[:digit:]]\+-{{OS}}-{{.NODEJS_ARCH}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but I noticed we could remove the / at the end of line 303

@kirkrodrigues kirkrodrigues merged commit ca9431b into y-scope:main May 3, 2024
2 checks passed
@kirkrodrigues kirkrodrigues deleted the lint-on-macos branch May 3, 2024 20:07
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