pawamoy / git-changelog

Automatic Changelog generator using Jinja2 templates.
https://pawamoy.github.io/git-changelog
ISC License
136 stars 34 forks source link

FEAT: Add configuration files #55

Closed oesteban closed 1 year ago

oesteban commented 1 year ago

Permit git-changelog be configured permanently via config files.

Resolves: #54.

oesteban commented 1 year ago

Okay, this could count as an MVP.

Unfortunately, I'm not sure I have the bandwidth to write some tests right now.

pawamoy commented 1 year ago

Thanks, I'll try to review it soon. I'll see if I can also add some tests :+1:

oesteban commented 1 year ago

Okay, I think this is getting there. However, I've hit some stomper with tests/test_cli.py::test_main. It passes locally on my workstation but doesn't on GHA.

I believe this test is actually just checking that the input is parsed and default settings are set, so I would just remove it (I don't think it covers anymore lines than the other tests).

The problem with the test is that it is checking two different things - whether the settings are properly parsed and then the call to main.

If the goal is to see that the parser is working, I suggest just testing parse_settings instead.

LMKWYT

pawamoy commented 1 year ago

It passes locally on my workstation but doesn't on GHA.

I'll take a look.

pawamoy commented 1 year ago

Actually the test failures I see in CI are weird. If you look at the second and third last runs, there are 2 or 3 tests failing, all because of the git config --get remote.origin.url subprocess. In the last run though, only one test fails on this. And this is completely unrelated to this PR. Let me re-run CI to see if it happens consistently.

pawamoy commented 1 year ago

OK, better. However we're hitting this: FutureWarning:bump-latest = falseoption found in config file (/tmp/pytest-of-runner/pytest-0/popen-gw0/test_settings_warning_False_0/.git-changelog.toml).

I think there's a race condition in the tests. We shouldn't create test files in the repository folder, but rather use a tmp directory, move there and only then test things. EDIT: oh but you're already doing that. Not sure what's happening. Will continue to investigate. EDIT 2: oh of course, os.chdir() affects the whole process, not just the current test.

pawamoy commented 1 year ago

I've fixed the tests, applied Black formatting, and finished fixing Mypy warnings. I'm ready to squash and merge, let me know if there's anything else you'd like to do before that. And thanks again for your amazing work :rocket:

oesteban commented 1 year ago

All good on my end. Thanks for the tool and for taking this PR over the finish line.

On Mon, Aug 21, 2023, 23:56 Timothée Mazzucotelli @.***> wrote:

I've fixed the tests, applied Black formatting, and finished fixing Mypy warnings. I'm ready to squash and merge, let me know if there's anything else you'd like to do before that. And thanks again for your amazing work 🚀

— Reply to this email directly, view it on GitHub https://github.com/pawamoy/git-changelog/pull/55#issuecomment-1687098875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAESDRQH4DODUD376JTQSU3XWPKSVANCNFSM6AAAAAA3VIU3MI . You are receiving this because you authored the thread.Message ID: @.***>

pawamoy commented 1 year ago

Good catch on the regular expressions written in TOML!