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

ci: enable commitlinter with conventional rules #138

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

artek-koltun
Copy link
Contributor

@artek-koltun artek-koltun commented Jul 18, 2023

please find conventional commit rules https://www.conventionalcommits.org/en/v1.0.0/

if: ${{ github.event.pull_request.merged != true }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 6: GitHub-owned GitHubAction not pinned by hash
Click Remediation section below to solve this issue
with:
fetch-depth: 0
- name: Setup Node
uses: actions/setup-node@v2

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 6: GitHub-owned GitHubAction not pinned by hash
Click Remediation section below to solve this issue
.github/workflows/commitlint.yml Fixed Show fixed Hide fixed
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #138 (b410982) into main (f751cc6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #138   +/-   ##
=======================================
  Coverage   62.06%   62.06%           
=======================================
  Files           1        1           
  Lines          58       58           
=======================================
  Hits           36       36           
  Misses         15       15           
  Partials        7        7           

@artek-koltun artek-koltun marked this pull request as ready for review July 18, 2023 13:25
@artek-koltun artek-koltun requested a review from a team as a code owner July 18, 2023 13:25
.github/workflows/linters.yml Fixed Show fixed Hide fixed
.github/workflows/linters.yml Fixed Show fixed Hide fixed
.github/workflows/linters.yml Fixed Show fixed Hide fixed
@artek-koltun artek-koltun force-pushed the ci-enable-commitlint branch 2 times, most recently from 9a66bc9 to aa12754 Compare July 18, 2023 13:31
node-version: 20.x
- name: Install dependencies
run: npm i -g @commitlint/cli @commitlint/config-conventional
- name: Validate all commits from PR

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 6: npmCommand not pinned by hash
Click Remediation section below to solve this issue
Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

@artek-koltun can we put .commitlintrc.json inside .github folder ?

Copy link

@seroyer seroyer left a comment

Choose a reason for hiding this comment

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

This is a great idea. I personally like even stricter rules than the defaults in commitlinter (like shorter headers), but I'm ok with this.

@artek-koltun
Copy link
Contributor Author

artek-koltun commented Jul 18, 2023

@artek-koltun can we put .commitlintrc.json inside .github folder ?

.commitlintrc.json is a config file like .golangci.yml which resides in the root. I assume for the purpose to run it locally without specifying the path to the config explicitly. If we are not going to support a case when commitlint is run locally, then we can move it into .github...

@glimchb
Copy link
Member

glimchb commented Jul 18, 2023

.commitlintrc.json is a config file like .golangci.yml which resides in the root.

I understand. for example renovate.json can eaither be in root or in .github folder. both locations are supported
see https://docs.renovatebot.com/configuration-options/

So I'm asking if .commitlintrc.json is supported in .github folder as well?

@artek-koltun
Copy link
Contributor Author

I understand. for example renovate.json can eaither be in root or in .github folder. both locations are supported see https://docs.renovatebot.com/configuration-options/

So I'm asking if .commitlintrc.json is supported in .github folder as well?

Checked it and the answer is no, it is not supported. It supports different config file formats, but they should be in the root location.
Probably yml format is preferable for us for consistency, since we already have .golangci.yml in the root...

@glimchb
Copy link
Member

glimchb commented Jul 19, 2023

Checked it and the answer is no, it is not supported. It supports different config file formats, but they should be in the root location. Probably yml format is preferable for us for consistency, since we already have .golangci.yml in the root...

thanks for checking

@glimchb glimchb merged commit c1facab into opiproject:main Jul 19, 2023
18 checks passed
@glimchb
Copy link
Member

glimchb commented Jul 19, 2023

@artek-koltun missed a problem in this PR, I fixed it here 95ce6d7

@glimchb
Copy link
Member

glimchb commented Jul 19, 2023

@artek-koltun new problem:

The following actions uses node12 which is deprecated and will be forced to run on node16: actions/setup-node@v2.

For more info: https://github.blog/changelog/2023-06-13-github-actions-all-actions-will-run-on-node16-instead-of-node12-by-default

@glimchb
Copy link
Member

glimchb commented Jul 19, 2023

See new PR #139

@artek-koltun
Copy link
Contributor Author

artek-koltun commented Jul 21, 2023

@artek-koltun missed a problem in this PR, I fixed it here 95ce6d7

if we just use

  push:
    branches: [ "main" ]

It will check all the commits from the very first one in the history and will always report fail status. But maybe with ahmadnassri/action-commit-lint it is going to be ok.

Another question if we need that check after merging PRs? Probably running it only on PRs and forbidding pushes into main is enough?

@artek-koltun
Copy link
Contributor Author

@artek-koltun new problem:

The following actions uses node12 which is deprecated and will be forced to run on node16: actions/setup-node@v2.

For more info: https://github.blog/changelog/2023-06-13-github-actions-all-actions-will-run-on-node16-instead-of-node12-by-default

Where did you see that warning? Looks like we use 20.x version...

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.

3 participants