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

Match config conventional with new spec [WIP] #767

Closed
wants to merge 3 commits into from

Conversation

byCedric
Copy link
Member

@byCedric byCedric commented Jul 28, 2019

Description

Fixes #658

Motivation and Context

Usage examples

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@byCedric byCedric changed the title New config conventional parser preset [WIP] Match config conventional with new spec [WIP] Jul 28, 2019
@bcoe
Copy link
Member

bcoe commented Aug 8, 2019

@byCedric we're starting to roll out commit linting more internally, so I'm excited for this work 😄

a couple things we should make sure we capture:

  1. I think commit lint is a bit stricter about type than we suggest on conventionalcommits.org, any type is fine (we're not as strict as angular).
  2. I really love the new feat!: syntax, as a short hand for indicating a breaking change; would love to make sure we support this.

@byCedric
Copy link
Member Author

byCedric commented Aug 9, 2019

Awesome! 🎉 And thanks for helping focussing on the important parts!

  1. I had the same idea about the types. Like I outlined here, the spec describing feat and fix is more directed at the developers rather than Commitlint right? 😁

  2. I think I saw the implementation for the shorthand in the conventional-changelog-conventionalcommits parser right? So switching the parser to this one should do the trick, right? Like outputting a parsed commit with breaking change notes, or do you think we still need to add something to make it work?

Right now I'm in Thailand, celebrating the engagement of a good friend of me. Maybe @marionebl or @escapedcat can lend a hand? When I get back I'll prioritize this a bit more 😁

@byCedric
Copy link
Member Author

byCedric commented Aug 9, 2019

One thing I thought of that we should properly "inform" developers using Commitlint when we open up the types. Maybe also provide a short upgrade guide to keep Commitlint as is for them (with the stricter types). But I think we can cover this fairly easy in the final stage, just before releasing 😁

@bcoe
Copy link
Member

bcoe commented Aug 9, 2019

@byCedric we do have the new parser conventional-changelog-conventionalcommits, I think if you use this it will capture the ! syntax 👍

As for being strict regarding types, I think I agree, we should just make sure it's well documented how a developer provides their own set of types, or turns off type enforcement as the case might be 👍

Have fun in Thailand, appreciate your hard work on this project. FYI, here's the bot I've been building in action:

https://github.com/googleapis/release-please/pull/222/checks?check_run_id=189188026

deployed from a framework we're working on here:

https://github.com/googleapis/repo-automation-bots/blob/master/packages/conventional-commit-lint/src/conventional-commit-lint.ts

@byCedric byCedric mentioned this pull request Sep 28, 2019
4 tasks
BREAKING CHANGE: this includes the new breaking change shorthand and is therefore probably a breaking change
@byCedric byCedric force-pushed the refactor/config-conventional branch from 9d7f0eb to 7c17ffc Compare October 6, 2019 19:13
The specification only provides feat and fix as a guideline, its not a hard limitation. Because of this, it should accept any type.
@byCedric
Copy link
Member Author

byCedric commented Oct 6, 2019

Let's keep this PR open for now. The type-enum should is removed (the spec doesn't forbid other types). Because of that, we need to consider this a breaking change. I suggest opening a new PR with only the updated parser preset, which should work with backward compatibility, to merge it without breaking change 😄

@marionebl
Copy link
Contributor

Discussed this between @byCedric @escapedcat and me. The current plan is to relax @commitlint/config-conventional as per spec.

That being said we will move the current, stricter set to @commitlint/config-conventional-strict and show that as config to use in the installation guides.

@escapedcat
Copy link
Member

After #821 did the most part of this I believe we're good here for now. Happy to re-open this if anyones has further feedback.

@escapedcat escapedcat closed this May 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

feat: update conventional commit linter
4 participants