scientific-python / action-towncrier-changelog

GitHub Action to check towncrier changelog
BSD 3-Clause "New" or "Revised" License
2 stars 5 forks source link

Ideas for improvements #1

Open pllim opened 3 years ago

pllim commented 3 years ago

If we ever want to come back to using this, some ideas:

larsoner commented 11 months ago

One thing I was considering adding to MNE-Python (when thinking about automating towncrier stuff) was something that took doc/changes/devel/enhancement.rst and automatically git mved it to doc/changes/devel/12345.enhancement.rst for example. Then as a dev I just create the file without the PR number and it pushes directly once I open the PR. Thoughts?

pllim commented 11 months ago

Then as a dev I just create the file without the PR number and it pushes directly once I open the PR. Thoughts?

That would require the token to have the power of writing to contributor's fork. I cannot remember on top of my head if the default token can do that, but it is technically possible since pre-commit.ci already does it (via comment).

The git history purist in me doesn't like this though because for each PR, you will always have a commit where the bot moves the file around. You will have to use the Squash And Merge button to squash it out, or do the squash manually. In the end, does it really save time/effort? Would also like to hear what @bsipocz , @saimn , and/or @Cadair think here.

Currently in astropy, we could predict the PR number using https://github.com/astropy/astropy-tools?tab=readme-ov-file#next_pr_numberpy (usually accurate unless someone else beat you to opening a new PR/issue between running the script and you opening that PR).

larsoner commented 11 months ago

That would require the token to have the power of writing to contributor's fork.

True, so we'd need to check what you say and (potentially) give instructions for how to add a write-allowed token (or whatever the process would be).

The git history purist in me doesn't like this though because for each PR, you will always have a commit where the bot moves the file around. You will have to use the Squash And Merge button to squash it out, or do the squash manually. In the end, does it really save time/effort?

At least in MNE-Python we always squash+merge, so for us this isn't a blocker, though I see how it could be for others. And we have the same problem for pre-commit or any other automated CI solution.

Currently in astropy, we could predict the PR number using

Neat! Still an automated solution might be nicer...

To me as long as the git mv is opt-in and its limitations clearly documented, it seems like it could be useful to others. But we can wait and see. I'm going to implement it at the MNE-Python end first either way, assuming we decide to actually go with towncrier -- we are kind of testing it out at the moment! Happy to move it here (or not) if it would help others (or wouldn't)...

bsipocz commented 11 months ago

If it's an opt-in, then sure.

But I would expect that there is an overlap within the population who cannot figure out the next PR number and those running into issue of the remote diverging from their branch. So if all this is generates more things to look out for newcomers, I would expect the net benefit is not positive. But then again, if it's opt-in and would be useful for your project, then I don't see why it cannot be supported.

bsipocz commented 11 months ago

(In astropy it's also very uncommon that one adds the changelog in their first commit/at the time of opening the PR, so in practice the PR number is well known by the time towncrier comes into the picture)