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

refactor: migration semantic prefix #16218

Merged

Conversation

RahulGautamSingh
Copy link
Collaborator

Changes

Context

  1. Migrate inline with same sort  #11459
  2. Refactor/migrations #12957

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

@RahulGautamSingh RahulGautamSingh marked this pull request as draft June 24, 2022 08:14
@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jun 24, 2022

Refactoring code logic using the SemanticCommitMessage methods.

@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jun 24, 2022

Refactoring code logic using the SemanticCommitMessage methods.

Idea dropped needs discussion.
The SemanticCommitMessage returns full prefix as type and returns null for scope ie. unreliable for now.
Eg. for fix(deps):* we get type: fix(deps) , scope:null instead of type: fix , scope:deps

@RahulGautamSingh RahulGautamSingh marked this pull request as ready for review June 24, 2022 08:41
@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jun 24, 2022

I am in doubt regarding keeping semanticPrefix in depreciated because @pret-a-porter didn't do it in his PR. But from the commit history it seemed we want it depreciated.

PS: I am unable to check the exact PR for migration of semanticPrefix to semanticCommitScope for some reason. If you could link it that would help.

@pret-a-porter
Copy link
Contributor

Refactoring code logic using the SemanticCommitMessage methods.

Idea dropped needs discussion. The SemanticCommitMessage returns full prefix as type and returns null for scope ie. unreliable for now. Eg. for fix(deps):* we get type: fix(deps) , scope:null instead of type: fix , scope:deps

fix(deps):* is not semantic commit message, because it is not follow to specification properly, therefore it will not be parsed by SemanticCommitMessage.fromString
https://www.conventionalcommits.org/en/v1.0.0/#specification
5. A description MUST immediately follow the colon and space after the type/scope prefix.

I still think would be better to use static method SemanticCommitMessage.fromString.

@RahulGautamSingh RahulGautamSingh dismissed a stale review via 348412d June 26, 2022 11:17
@viceice viceice marked this pull request as draft June 27, 2022 20:05
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.

I think this is now blocked, as we hade again issues with that class based semantic commit parsing

@pret-a-porter
Copy link
Contributor

I think this is now blocked, as we hade again issues with that class based semantic commit parsing

What is the issue?

@viceice
Copy link
Member

viceice commented Jun 28, 2022

I think this is now blocked, as we hade again issues with that class based semantic commit parsing

What is the issue?

@RahulGautamSingh RahulGautamSingh marked this pull request as ready for review July 1, 2022 10:47
@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jul 1, 2022

I have reverted back to using old logic. We can modify it to use SemanticCommitMessage in future, when it's ready.

@RahulGautamSingh RahulGautamSingh requested review from viceice and pret-a-porter and removed request for pret-a-porter July 1, 2022 10:49
@pret-a-porter
Copy link
Contributor

I think this is now blocked, as we hade again issues with that class based semantic commit parsing

What is the issue?

Seems like issue in generate script, not in Semantic Commit Message class

@rarkins rarkins enabled auto-merge (squash) July 6, 2022 02:22
@rarkins rarkins merged commit 895218d into renovatebot:main Jul 6, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.105.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 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.

None yet

5 participants