Handle deletion of uncommitted news fragments

[abompard]

Before this commit, all the news fragments needed to be committed into git, or the fragments removal after building the news file would crash.

In my workflow, I add missing fragments before building the news file because I'm extracting author names from the git log, and towncrier crashes at the end of the build process.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.80%. Comparing base (70da86f) to head (cb08709).
Report is 72 commits behind head on trunk.

❗ Current head cb08709 differs from pull request most recent head 3ea6861

Please upload reports for the commit 3ea6861 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk     #357      +/-   ##
==========================================
- Coverage   99.84%   92.80%   -7.05%     
==========================================
  Files          13       11       -2     
  Lines         631      500     -131     
  Branches      146      101      -45     
==========================================
- Hits          630      464     -166     
- Misses          0       18      +18     
- Partials        1       18      +17     

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

[adiroiban]

Many thanks for the changes.

They look good.

I left a few minor comments regarding the documentation/docstrings of these changes.

I would like to keep the _run helper private and outside of the git code :)

Sorry for the late review.

I hope you will still have time to drive this PR.

Let me know if you are no longer interested in working on it and I will try to take over and finalize this PR.

Cheers

[jaraco]

It looks like this issue has gotten a little stale, and since we haven't heard back from the contributor, I'm going to close it for now. If you'd like to revive the effort, feel free to ping this PR and we can re-open, or start a new one. Thanks.

[abompard]

Hey folks! Sorry I had totally forgotten about this PR. I'll try to find some time to finalize it, if it's still relevant.

[abompard]

Could you please reopen @jaraco ?

[adiroiban]

Thanks for the update.

Can you please fix the conflicts and update the docstring for run to describe it's current behaviour

We no longer support python 2.7

[abompard]

I'm more than happy to remove Python 2.7 remnants :-D
Alright, it's updated and tests should pass now.

[adiroiban]

Thanks for the changes.

I think that we are very close to being able to merge this.

But this needs more cleanup.

The commented lines in the test file are a blocker.

Thanks again.

[abompard]

I've made the changes, thanks a lot for your patience. FYI I do appreciate the in-depth reviews, even for minor style changes. I'm frequently on the other end of those ;-)
So thanks.

[SmileyChris]

Currently, the command line docs say:

By default, the processed news fragments are removed using git, which will also remove the fragments directory if now empty.

This should be updated. Maybe something like

By default, the processed news fragments are removed. For any fragments committed in your git repository, git rm will be used (which will also remove the fragments directory if now empty).

Chris is looking into this

[adiroiban]

Thanks for the update. I will let Chris do the final review and approval for this PR.

@abompard Note that we merge with squash.
There is not much use in rewriting the history for the PR branch.
Rewriting/amending the commits on the PR make it harder to do a followup for a review.

Cheers

[abompard]

@abompard Note that we merge with squash. There is not much use in rewriting the history for the PR branch. Rewriting/amending the commits on the PR make it harder to do a followup for a review.

Ah, sorry about that, I didn't know.

FYI, I've had this same issue many times (trying to review PRs that rebase/rewrite) and I ended up writing a small tool to help me with the workflow: https://pypi.org/project/git-pr-branch/. The main idea is that it will download the different revisions of a PR in different branches that you can diff, even if it's always 1 commit that's being rebased.
Maybe you'll find it useful for your workflow as well.

[adiroiban]

No problem.
I am looking after towncrier as my 4th job ... so I don't have much time for it.
This is why, using the embedded GitHub web tools makes it easier to review.

For most of the reviews, I don't checkout the code on my computer.
I am just doing a code review and hope the automated tests are doing their job.

[adiroiban]

needs-review

[adiroiban]

This looks good to me.
I will wait a bit more for Chris to review this.

If there is a big delay, we can have this merged.

There is no rush, as there is no relesed planed anytime soon...
but we might want do to a new release soon :)

Reviewed by Adi

[adiroiban]

I think that the comments from Chris were address.
I have merged this.

Thanks for your help!