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: commit message #13197

Merged
merged 38 commits into from
Jun 13, 2022
Merged

fix: commit message #13197

merged 38 commits into from
Jun 13, 2022

Conversation

pret-a-porter
Copy link
Contributor

Changes:

  1. Implemented abstract class CommitMessage
  2. Implemented class CustomCommitMessage
  3. Implemented class SemanticCommitMessage
  4. Implemented factory CommitMessageFactory for creating instance of SemanticCommitMessage or CustomCommitMessage by provided shared renovate config
  5. Logic of generating PR title improved by using CommitMessageFactory

Context:

Fixes #12062

Documentation

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@pret-a-porter pret-a-porter changed the title Fix/commit message fix: commit message Dec 19, 2021
@viceice viceice self-requested a review December 19, 2021 14:31
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/workers/repository/model/commit-message-factory.ts Outdated Show resolved Hide resolved
lib/workers/repository/onboarding/branch/commit-message.ts Outdated Show resolved Hide resolved
lib/workers/repository/model/semantic-commit-message.ts Outdated Show resolved Hide resolved
lib/workers/repository/model/semantic-commit-message.ts Outdated Show resolved Hide resolved
@rarkins
Copy link
Collaborator

rarkins commented Dec 30, 2021

I feel concerned when there's so many changes to both implementation and tests at the same time - it's much easier for us to miss a regression test. @pret-a-porter can you split this, e.g. to change the tests part first with minimal changes to the implementation?

@pret-a-porter
Copy link
Contributor Author

I feel concerned when there's so many changes to both implementation and tests at the same time - it's much easier for us to miss a regression test. @pret-a-porter can you split this, e.g. to change the tests part first with minimal changes to the implementation?

Yeah, I can. It can be two next PR's:

  1. Refactoring CommitMessage class and introducing new SemanticCommitMessage class
  2. Changing generateBranchConfig functionality

@pret-a-porter
Copy link
Contributor Author

@rarkins I have extracted #13328

@viceice viceice marked this pull request as draft January 4, 2022 10:29
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

blocked by #13328

@pret-a-porter pret-a-porter requested a review from viceice May 23, 2022 19:46
rarkins
rarkins previously approved these changes May 24, 2022
@pret-a-porter pret-a-porter requested a review from viceice June 11, 2022 16:21
@viceice viceice enabled auto-merge (squash) June 11, 2022 17:29
@viceice viceice merged commit ed639b4 into renovatebot:main Jun 13, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.84.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

rarkins added a commit that referenced this pull request Jun 13, 2022
rarkins added a commit that referenced this pull request Jun 13, 2022
Revert "fix: commit message (#13197)"

This reverts commit ed639b4.
MaronHatoum pushed a commit to MaronHatoum/renovate that referenced this pull request Jun 14, 2022
Revert "fix: commit message (renovatebot#13197)"

This reverts commit ed639b4.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 14, 2022
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.

Commit messages should not be fully lowercased
4 participants