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

chore(ci): parse commits correctly #294

Merged
merged 3 commits into from
Mar 25, 2022
Merged

chore(ci): parse commits correctly #294

merged 3 commits into from
Mar 25, 2022

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Mar 24, 2022

🧭 What and Why

Changes included:

  • git log --oneline gives SHA1 in 8 length. Previously the code and the test were written, assuming it was 7, which was wrong.

🧪 Test

CI

@netlify
Copy link

netlify bot commented Mar 24, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit 6940b74
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/623d7db2b2024a00083d9ce5

@eunjae-lee eunjae-lee requested review from a team, damcou and shortcuts and removed request for a team March 24, 2022 16:52
let message = commit.slice(8);
const LENGTH_SHA1 = 8;
const hash = commit.slice(0, LENGTH_SHA1);
let message = commit.slice(LENGTH_SHA1 + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the code just below, the CI allows empty scope for chores and docs, it will trigger the error line 68 I think.
Either we always enforce scope or you check here for the type first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, but chore and docs commit don't affect versioning. So it's okay to ignore them. It returns { error: '...' } but later the script just prints out the warning messages. (We are planning to include these ignored commits within the release issue in a separate PR)

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh sorry I didn't see the rest of the code, all good then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries. Thanks for the comment!

scripts/release/__tests__/create-release-issue.test.ts Outdated Show resolved Hide resolved
@eunjae-lee eunjae-lee enabled auto-merge (squash) March 24, 2022 17:13
@shortcuts
Copy link
Member

shortcuts commented Mar 24, 2022

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the generated/main branch.

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