Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

feat: support scoped commits #36

Closed
wants to merge 2 commits into from
Closed

feat: support scoped commits #36

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 18, 2021

Fixes #31

Currently the prefix can contain both type and scope but it's treated as type only. So I've added a step to parse those out. The scope is used in the line description similar to how they show up in conventional release notes.

I had to include tslib as to target ES2018 and use various syntax (like destructuring), it's needed.

src/commit.ts:75:15 - error TS2354: This syntax requires an imported helper but module 'tslib' cannot be found.

75       const { type, scope } = getTypeAndScope(words[1])

Downgrading the target to ES2015 avoids the error. I guess they include helpers automatically in ES2015 and stopped doing that in later versions.

I've also committed changes in the dist folder as that seems to be convention here.

Since the tsbuildinfo is on 3.8.3, I made a separate commit to upgrade to 3.9.9 which allows us to use the latest tslib. I can split this into a separate PR if needed.

@ghost ghost requested a review from Shesekino as a code owner May 18, 2021 11:37
@@ -1299,6 +1299,7 @@ module.exports = {
"use strict";

Object.defineProperty(exports, "__esModule", { value: true });
exports.previewFromCommits = void 0;
Copy link
Author

@ghost ghost May 18, 2021

Choose a reason for hiding this comment

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

TS 3.9.9 output change. It defines the export first then assigns it later.

if (!matches) {
// Malformed commit prefix. Try our best.
return {
type: prefix.split(':')[0] || ''
Copy link
Author

Choose a reason for hiding this comment

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

I've kept this logic from previous behaviour in case some existing user is reliant on it.

scope ? `${scope}:` : undefined,
rest,
`(${hash})`
].filter(part => !!part).join(' ');
Copy link
Author

Choose a reason for hiding this comment

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

I've split these sections up as scope is optional and a ternary inside a template literal is confusing.

Copy link
Contributor

@Shesekino Shesekino left a comment

Choose a reason for hiding this comment

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

Didn't review this thoroughly, but I rather not block this feature :) Thanks!

@Shesekino
Copy link
Contributor

@jahed-snyk would you be able to reopen the PR from a branch on this repo rather than a fork?

@ghost
Copy link
Author

ghost commented Jun 1, 2021

@Shesekino will do, I assumed I didn't have write-access. Is there a testing process for this? If not, I'll try this out on a test repo before merging just to confirm everything's working.

@Shesekino
Copy link
Contributor

A test repo sounds fantastic 👌
We were missing permissions on the dev group - I've modified that :) Let me know if you're missing anything please 🙏

@ghost
Copy link
Author

ghost commented Aug 12, 2021

There's a good chance I won't have time for this... So closing for now. If anyone else wants to pick it up feel free to.

@ghost ghost closed this Aug 12, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

release-notes-preview gets broken by the <scope> portion of a commit header
1 participant