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

feat: allow git commit type to be changed in configuration #462

Merged
merged 3 commits into from
May 24, 2023

Conversation

tobiasbueschel
Copy link
Contributor

@tobiasbueschel tobiasbueschel commented Apr 17, 2023

What: This allows users of the All Contributors app to configure the Git commit tag that the tool should use to add new contributors.

Why: Adding this will help users to customize the Git commit tag, which can be helpful in situations where you don't want that All Contributors changes appear in an auto-generated changelog that considers e.g., all docs commits.

Instead, one can overwrite it to be chore and therefore the All Contributors changes wouldn't appear in a changelog. One could argue that only features or bug fixes should appear in changelogs, but I have a few use cases where we're developing documentation apps using Docusaurus and configured semantic-release to also consider any docs commit tags for the changelog.

How: I've followed a similar pattern that was used for commitConvention and introduced commitType which is configurable.

Checklist:

  • [?] Documentation => I've had a look through the repo but didn't find any documentation on commitConvention and so also didn't add one for commitType
  • Ready to be merged => yes, but would be great to have someone else test this flow
  • Added myself to contributors table.
    Bot Usage

Do let me know if there's anything else I shall change in this PR? :)

Thanks

@tobiasbueschel tobiasbueschel requested a review from a team as a code owner April 17, 2023 16:43
@vercel
Copy link

vercel bot commented Apr 17, 2023

@tobiasbueschel is attempting to deploy a commit to the All Contributors Team on Vercel.

A member of the Team first needs to authorize it.

lib/modules/config.js Outdated Show resolved Hide resolved
@tobiasbueschel tobiasbueschel changed the title feat: allow git commit tag to be changed in configuration feat: allow git commit ty[e to be changed in configuration May 19, 2023
@tobiasbueschel tobiasbueschel changed the title feat: allow git commit ty[e to be changed in configuration feat: allow git commit type to be changed in configuration May 19, 2023
Copy link
Member

@Berkmann18 Berkmann18 left a comment

Choose a reason for hiding this comment

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

That looks good to me.

@vercel
Copy link

vercel bot commented May 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
app ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2023 10:46pm

@gr2m
Copy link
Contributor

gr2m commented May 20, 2023

Not sure why the tests are failing 🤔

But I pointed https://github.com/apps/all-contributors-preview to the preview deployment. Can you give it a try on a test repository to see if it all works as expected? Make sure you don't have https://github.com/apps/allcontributors already installed in the same repo

@tobiasbueschel
Copy link
Contributor Author

Not sure why the tests are failing 🤔

But I pointed github.com/apps/all-contributors-preview to the preview deployment. Can you give it a try on a test repository to see if it all works as expected? Make sure you don't have github.com/apps/allcontributors already installed in the same repo

I've updated the tests which should also fix the deployed version - sorry for the oversight as I missed updating one variable! Here's the test repository, once the new version is deployed I can test it :)

https://github.com/tobiasbueschel/all-contributors-test

@codecov
Copy link

codecov bot commented May 20, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c733295) 100.00% compared to head (c03d7b7) 100.00%.

❗ Current head c03d7b7 differs from pull request most recent head ba0aab7. Consider uploading reports for the commit ba0aab7 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #462   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines          451       453    +2     
  Branches        53        54    +1     
=========================================
+ Hits           451       453    +2     
Impacted Files Coverage Δ
lib/add-contributor.js 100.00% <100.00%> (ø)
lib/modules/config.js 100.00% <100.00%> (ø)
lib/modules/repository.js 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@gr2m
Copy link
Contributor

gr2m commented May 20, 2023

once the new version is deployed I can test it

ready 👍🏼

@tobiasbueschel
Copy link
Contributor Author

once the new version is deployed I can test it

I'll have to ask you to redeploy again :) Glad that we tested as I found that the PR title updated with the correct commit type, but the two commits that were push as part of the PR still had the old commit type.
image

I've pushed a fix to update it - and now it should work fully :)

});
}
}

async createOrUpdateFiles({ filesByPath, branchName, convention }) {
async createOrUpdateFiles({ filesByPath, branchName, convention, commitType }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On that note, we're handing down a lot of config like variables to these class methods. Perhaps in a later PR one could refactor this so that we set up the branchName, convention, commitType etc. when the Repository class is instantiated.

@gr2m
Copy link
Contributor

gr2m commented May 21, 2023

I found that the PR title updated with the correct commit type, but the two commits that were push as part of the PR still had the old commit type

Great catch!

I updated the webhook URL. I actually found that there is URL that remains the same for this pull request:https://app-git-fork-tobiasbueschel-master-all-contributors.vercel.app/

Next time I'll know :D

@tobiasbueschel
Copy link
Contributor Author

Thanks again @gr2m the demo repository works now: tobiasbueschel/all-contributors-test#4

@gr2m gr2m merged commit dbb96f3 into all-contributors:master May 24, 2023
3 checks passed
@gr2m
Copy link
Contributor

gr2m commented May 24, 2023

@all-contributors please add @tobiasbueschel for code and testing

@allcontributors
Copy link
Contributor

@gr2m

I've put up a pull request to add @tobiasbueschel! 🎉

@github-actions
Copy link

🎉 This PR is included in version 1.19.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants