Skip to content

feat(cli): add config option to specify config file path #1047

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

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

rockleona
Copy link
Contributor

@rockleona rockleona commented Apr 1, 2024

Description

CLI now have an option --config can specify the config file path,
if the config file is not in root folder, user can specify the located file as well.

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

cz now have an option to specify config file,
if the given file is existed, will only read configuration from the given file,
else, a error would be raise since the given file do not exist.

Steps to Test This Pull Request

Execute any cz command with --config option, if the config file do not exist, a error would raised.

Additional context

resolve #656

Copy link

codecov bot commented Apr 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.50%. Comparing base (120d514) to head (bf6a6bf).
Report is 260 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1047      +/-   ##
==========================================
+ Coverage   97.33%   97.50%   +0.16%     
==========================================
  Files          42       55      +13     
  Lines        2104     2446     +342     
==========================================
+ Hits         2048     2385     +337     
- Misses         56       61       +5     
Flag Coverage Δ
unittests 97.50% <100.00%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rockleona rockleona marked this pull request as ready for review April 1, 2024 15:15
@woile
Copy link
Member

woile commented Apr 2, 2024

What's the motivation for this change?

@rockleona
Copy link
Contributor Author

What's the motivation for this change?

Simply just found out #656 is quite interesting for me,
but actually I can not imagine about the usage or in what scenario user will use this option.

@Lee-W
Copy link
Member

Lee-W commented Apr 2, 2024

What's the motivation for this change?

Simply just found out #656 is quite interesting for me, but actually I can not imagine about the usage or in what scenario user will use this option.

Someone who wants to use a single config for all projects might need it. or mono repo maybe?

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Hi @rockleona, thanks so much for implementing this. I left a few comments.

@rockleona rockleona requested a review from Lee-W April 6, 2024 16:20
Copy link
Member

@Lee-W Lee-W 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 we're really close to merging! In addition to the comment to address, I would suggest you squash the latest 2 commits into the first one as they're minor fixes and do not really need to appear in the changelog. Thanks!

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

LGTM. @woile @noirbizarre I'll merge this one these days. Please let me know if you want to take a deeper look

@Lee-W Lee-W added pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check and removed pr-status: wait-for-review pr-status: wait-for-modification labels Apr 9, 2024
Copy link
Member

@noirbizarre noirbizarre left a comment

Choose a reason for hiding this comment

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

Good to me.

Non blocking suggestion: test and ensure that user home path expansion works (users WILL use home-relative forms for sure so handling cases like ~/path/to/config from start will avoid inevitable issues)

@Lee-W
Copy link
Member

Lee-W commented Apr 11, 2024

Good to me.

Non blocking suggestion: test and ensure that user home path expansion works (users WILL use home-relative forms for sure so handling cases like ~/path/to/config from start will avoid inevitable issues)

Tested on my end. Works fine. Gonna merge it 🚀

@Lee-W Lee-W merged commit a47bdd0 into commitizen-tools:master Apr 11, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option --config to set the pyproject.toml configuration file path.
4 participants