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

fix(commitlint): skip task on master #3650

Merged

Conversation

chrisbottin
Copy link
Contributor

The rule between master and branch should be different to take the PR reference in consideration when merging.


This PR should be fixing the current build failure on master due to the latest commit message fix: report launcher process error when exit event is not emitted (#3647) being more than 72 characters (https://travis-ci.org/github/karma-runner/karma/jobs/757457475).


Problem

The problem is that the same header-max-length rule is applied to master and the PR branches and when a PR branch is merged the commit message has the PR reference appended to it making the length longer and potentially breaking the commitlint on master

Proposed Solution

Using different header-max-length rules for master and branches.

  • header-max-length rule on master set to 80 characters to temporary fix the build on master and then submit another PR to set the header-max-length rule on master set to 72 characters.
  • header-max-length rule on branches set to 64 characters (72 - 8 characters for the PR reference)

@devoto13
Copy link
Collaborator

devoto13 commented Feb 8, 2021

Hm, maybe it is better to simply disable commit message validation on master? 🤔

@AppVeyorBot
Copy link

Build karma 2926 completed (commit c076f6b558 by @chrisbottin)

@karmarunnerbot
Copy link
Member

Build karma 529 completed (commit c076f6b558 by @chrisbottin)

@karmarunnerbot
Copy link
Member

Build karma 528 completed (commit c076f6b558 by @chrisbottin)

@chrisbottin
Copy link
Contributor Author

Hm, maybe it is better to simply disable commit message validation on master? 🤔

@devoto13 yes we could do that but do you mean permanently? If we are happy with having no commitlint running on master then we can just disable it (I can submit another PR) ... otherwise we do need this PR to have different header-max-length rules.

Let me know

@devoto13
Copy link
Collaborator

devoto13 commented Feb 8, 2021

Yes, permanently. If you think about it, the check is not really actionable: one can't revert or fix the commit in master branch. So we can only run it in PRs/branches where we can prevent a bad commit message. But it's just my opinion. It's up to @johnjbarton to decide which approach to use.

@johnjbarton
Copy link
Contributor

Yes @devoto13 is right, I like the no-master solution better. @chrisbottin can you do it?

@chrisbottin chrisbottin changed the title fix(commitlint): change header-max-length rule fix(commitlint): skip task on master Feb 13, 2021
@karmarunnerbot
Copy link
Member

Build karma 535 completed (commit 138b283a52 by @chrisbottin)

@AppVeyorBot
Copy link

Build karma 2932 completed (commit 138b283a52 by @chrisbottin)

@karmarunnerbot
Copy link
Member

Build karma 534 completed (commit 138b283a52 by @chrisbottin)

@AppVeyorBot
Copy link

Build karma 2933 completed (commit 339ac71f89 by @chrisbottin)

@karmarunnerbot
Copy link
Member

Build karma 536 completed (commit 339ac71f89 by @chrisbottin)

@karmarunnerbot
Copy link
Member

Build karma 535 completed (commit 339ac71f89 by @chrisbottin)

@chrisbottin
Copy link
Contributor Author

@johnjbarton done as requested.

Copy link
Contributor

@johnjbarton johnjbarton left a comment

Choose a reason for hiding this comment

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

Thanks, sorry for the delay lost track of this one.

@johnjbarton johnjbarton merged commit 3fc6fda into karma-runner:master Mar 9, 2021
karmarunnerbot pushed a commit that referenced this pull request Mar 9, 2021
## [6.1.2](v6.1.1...v6.1.2) (2021-03-09)

### Bug Fixes

* **commitlint:** skip task on master ([#3650](#3650)) ([3fc6fda](3fc6fda))
* patch karma to allow loading virtual packages ([#3663](#3663)) ([5bfcf5f](5bfcf5f))
@karmarunnerbot
Copy link
Member

🎉 This PR is included in version 6.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

This was referenced Mar 15, 2021
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request May 16, 2023
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request May 16, 2023
## [6.1.2](karma-runner/karma@v6.1.1...v6.1.2) (2021-03-09)

### Bug Fixes

* **commitlint:** skip task on master ([karma-runner#3650](karma-runner#3650)) ([3fc6fda](karma-runner@3fc6fda))
* patch karma to allow loading virtual packages ([karma-runner#3663](karma-runner#3663)) ([5bfcf5f](karma-runner@5bfcf5f))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants