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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/clp-lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ concurrency:

jobs:
lint-check:
runs-on: "ubuntu-latest"
strategy:
matrix:
os: ["macos-latest", "ubuntu-latest"]
runs-on: "${{matrix.os}}"
steps:
- uses: "actions/checkout@v3"
with:
Expand All @@ -23,6 +26,10 @@ jobs:
shell: "bash"
run: "npm install -g @go-task/cli"

- if: "matrix.os == 'macos-latest'"
name: "Install coreutils (for md5sum)"
run: "brew install coreutils"

- name: "Run lint task"
shell: "bash"
run: "task lint:check"
9 changes: 5 additions & 4 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,8 @@ tasks:
curl --header "Cache-Control: no-cache, no-store" --silent "{{.NODEJS_VERSION_BASE_URL}}"
| grep
--only-matching
--perl-regexp
--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

| head --lines 1
cmds:
- task: "download-and-extract-tar"
Expand Down Expand Up @@ -565,8 +564,10 @@ tasks:
vars: ["FILE_PATH", "SED_EXP"]
cmds:
- |-
# NOTE: We can't use `sed -i` since `-i` has different syntax on Linux and macOS
# NOTE:
# 1. We can't use `sed -i` since `-i` has different syntax on Linux and macOS
# 2. We can't use `--regexp` instead of `-E` since `--regexp` is not supported on macOS
src="{{.FILE_PATH}}"
dst="{{.FILE_PATH}}.tmp"
sed --regexp-extended '{{.SED_EXP}}' "${src}" > "${dst}"
sed -E '{{.SED_EXP}}' "${src}" > "${dst}"
mv "${dst}" "${src}"
20 changes: 18 additions & 2 deletions docs/src/dev-guide/contributing-linting.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,23 @@
# Linting

Before submitting a PR, ensure you've run our linting tools and either fixed any violations or
suppressed the warning. To run our linting workflows locally, you'll need [Task]. Alternatively,
you can run the [clp-lint] workflow in your fork.
suppressed the warning. If you can't run the linting workflows locally, you can enable and run the
[clp-lint] workflow in your fork.

## Requirements

We currently support running our linting tools on Linux and macOS. If you're developing on another
OS, you can submit a [feature request][feature-req], or use our [clp-lint] workflow in your fork.

To run the linting tools, besides commonly installed tools like `tar`, you'll need:

* `curl`
* `md5sum`
* Python 3.8 or newer
* python3-venv
* [Task]

## Running the linters

To perform the linting checks:

Expand All @@ -17,4 +32,5 @@ task lint:fix
```

[clp-lint]: https://github.com/y-scope/clp/blob/main/.github/workflows/clp-lint.yaml
[feature-req]: https://github.com/y-scope/clp/issues/new?assignees=&labels=enhancement&projects=&template=feature-request.yml
[Task]: https://taskfile.dev/
Loading