justinmayer / kagi

WebAuthn security keys and TOTP multi-factor authentication for Django
BSD 2-Clause "Simplified" License
91 stars 10 forks source link

Added support for trusted publishers. #73

Closed apollo13 closed 1 year ago

apollo13 commented 1 year ago

@justinmayer this will be a lot to review, I'll leave a few comments inline so you can chime in and suggest how we should do it properly.

apollo13 commented 1 year ago

You can see the generated metadata at https://github.com/apollo13/kagi/actions/runs/5154225573 I think it is rather nice to be able to inspect the wheel contents / metadata etc before uploading.

MarkusH commented 1 year ago

It looks like the wheel contains the tests: https://github.com/apollo13/kagi/actions/runs/5154225573/jobs/9282424241#step:3:245. I think, we should exclude the test files form the package.

apollo13 commented 1 year ago

@MarkusH for that we'd probably have to move them out of the package unless we want to work against the build tooling?

MarkusH commented 1 year ago

@apollo13 I would mind moving them. Back in the setup.py times, one could exclude files with a specific pattern. Does that not exist anymore?

justinmayer commented 1 year ago

It should be possible to exclude the tests from the build by adding something like the following to pyproject.toml:

[tool.poetry]
[…]
exclude = [
    "kagi/tests"
]
apollo13 commented 1 year ago

This change is getting larger than I'd like it to be. I'll work on a new minimal set that will allow us to use trusted publishers and we can add the other improvements later on. Hynek gave me a bit to think about so I am currently playing with tools like https://github.com/your-tools/tbump (for release bumping & tagging) as well as towncrier for the changelog (cause I like the idea of adding the changelog immediately and not as an afterthought).

I am sorry for closing this, but I guess it gave us all a bit to think about and maybe we will revive parts in the future? Expect the new PR today…

justinmayer commented 1 year ago

I also like recording changelog entries as part of contributions and not as an afterthought, which is precisely why AutoPub does just that via the RELEASE file. The project page discusses this in more detail: https://justinmayer.com/projects/autopub/

I rather like how AutoPub allows maintainers to issue releases merely by merging pull requests, and I'd like to continue improving AutoPub to add support for trusted publishers as well as work out any obstacles that inhibit alternative use cases.

apollo13 commented 1 year ago

I also like recording changelog entries as part of contributions and not as an afterthought, which is precisely why AutoPub does just that via the RELEASE file.

Mhm I must have missed something then. I thought as soon as a RELEASE.md file is there this triggers a release. Or differently put: if you have three pull requests, how do you submit a changelog entry for all of them without having to do a release for each of them?

I rather like how AutoPub allows maintainers to issue releases merely by merging pull requests, and I'd like to continue improving AutoPub to add support for trusted publishers as well as work out any obstacles that inhibit alternative use cases.

Understood, and I fully accept that. But I will focus more on kagi itself than improving autopub.

apollo13 commented 1 year ago

It should be possible to exclude the tests from the build by adding something like the following to pyproject.toml:

This would exclude the tests from the source package as well though (unless I made a mistake when testing locally).

justinmayer commented 1 year ago

This would exclude the tests from the source package as well though (unless I made a mistake when testing locally).

Related issue: https://github.com/python-poetry/poetry/issues/3380