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

✨ Add semver property to the gitmojis #692

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

ilyasmez
Copy link
Contributor

@ilyasmez ilyasmez commented Feb 2, 2021

Closes: #429

Description

What?

This PR adds a semver property to each Gitmoji. The semver property can be 'major' for breaking changes, 'minor' for new features, 'patch' for bugfixes/small improvements and null if the commit doesn't affect the version of the package.

Why?

Gitmojis are too granular, and we need a higher level grouping to know what kind of effect does each gitmoji/commit have on the project, which is necessary to be able to automate the package publishing process and so on.
Right now, there are some third-party projects that try to do this grouping on their side, but because there is a big room of interpretation of each emoji, they are not consistent.

Things to keep in mind

  • This is my interpretation of the gitmojis, please drop your comments, so we find a common ground.
  • The semantic versioning effect is based on the final package, and not the repo itself, so 🚚 for instance has no effect from my point of view, unless it affects the final package, and in that case the developer should use 💥 instead.

Tests

  • All tests passed.

@vercel
Copy link

vercel bot commented Feb 2, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carloscuesta/gitmoji/mpuf5jfqs
✅ Preview: https://gitmoji-git-fork-ilyasmez-feature-semver-property.carloscuesta.vercel.app

@ilyasmez
Copy link
Contributor Author

ilyasmez commented Feb 2, 2021

@elliot-nelson @gustavopch @frinyvonnick @vhoyer @carloscuesta
Could you please have a look?

Copy link
Owner

@carloscuesta carloscuesta left a comment

Choose a reason for hiding this comment

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

Awesome! Looks good to me, let's just wait for the opinion of other people that participated on the issue before merging this 🚀

Copy link
Collaborator

@vhoyer vhoyer left a comment

Choose a reason for hiding this comment

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

looks sweet to me; let's just hit the nail on which semver to use which what emoji, but the implementation looks flawless in my opinion :D

Copy link
Contributor

@frinyvonnick frinyvonnick left a comment

Choose a reason for hiding this comment

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

Looks good to me too 🎉

@ilyasmez
Copy link
Contributor Author

ilyasmez commented Feb 2, 2021

43f1bd0: @vhoyer had some valid arguments regarding ➕ and ➖, I've set them to "patch" as agreed on here.

@carloscuesta carloscuesta merged commit 832f58c into carloscuesta:master Feb 3, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2021
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.

Add a "semver" field for each emoji
4 participants