-
Notifications
You must be signed in to change notification settings - Fork 87
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
feat: Support generating change files from conventional commits #513
base: master
Are you sure you want to change the base?
Changes from all commits
8560dbd
da9760f
5261efb
a0bb896
0de404a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "minor", | ||
"comment": "Support generating change files from conventional commits", | ||
"packageName": "beachball", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { parseConventionalCommit } from '../../changefile/conventionalCommits'; | ||
|
||
describe.each<[string, ReturnType<typeof parseConventionalCommit>]>([ | ||
['fix: change message\nbody', { type: 'patch', message: 'change message' }], | ||
['chore: change', { type: 'none', message: 'change' }], | ||
['feat: change', { type: 'minor', message: 'change' }], | ||
['feat(scope): change', { type: 'minor', message: 'change' }], | ||
['feat!: change', { type: 'major', message: 'change' }], | ||
['feat(scope)!: change', { type: 'major', message: 'change' }], | ||
['foo', undefined], | ||
['fix(foo-bar): change', { type: 'patch', message: 'change' }], | ||
])('parse(%s)', (s, expected) => { | ||
test('should parse correctly', () => expect(parseConventionalCommit(s)).toEqual(expected)); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import { ChangeType } from '../types/ChangeInfo'; | ||
|
||
/** | ||
* 1. type | ||
* 2. scope | ||
* 3. breaking | ||
* 4. message | ||
*/ | ||
const COMMIT_RE = /([a-z]+)(?:\(([a-z\-]+)\))?(!)?: (.+)/i; | ||
|
||
interface ConventionalCommit { | ||
type: string; | ||
scope?: string; | ||
breaking: boolean; | ||
message: string; | ||
} | ||
|
||
interface Change { | ||
type: ChangeType; | ||
message: string; | ||
} | ||
|
||
export function parseConventionalCommit(commitMessage: string): Change | undefined { | ||
const match = commitMessage.match(COMMIT_RE); | ||
const data: ConventionalCommit | undefined = match | ||
? { type: match[1], scope: match[2], breaking: !!match[3], message: match[4] } | ||
: undefined; | ||
return data && map(data); | ||
} | ||
|
||
function map(d: ConventionalCommit): Change | undefined { | ||
if (d.breaking) { | ||
return { type: 'major', message: d.message }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a special case here, for semver 0.x.y packages. In that scheme breaking changes are a minor bump, and fixes/feats are a patch bump. npm, dependabot, are aware of that scheme, it it's used in some common packages like react-native. |
||
} | ||
|
||
switch (d.type) { | ||
case 'chore': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would Also, can you add support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tido64 I've been doing some hacking locally to get it "just right" and I think what would be best would be to let the user provide a dictionary of allowed types and the change type (major/minor/patch/none) it should map to. That has worked well for me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that makes sense. Also, I just saw your comment now, asking the exact same question 🤣 |
||
return { type: 'none', message: d.message }; | ||
case 'fix': | ||
return { type: 'patch', message: d.message }; | ||
case 'feat': | ||
return { type: 'minor', message: d.message }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this intersect with prerelease packages? |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: consider doing case insensitive match