Refactor _writer & bring to 100% coverage

[hynek]

I've been trying to understand _writer.py and while doing so, I've refactored it into something that fits my brain better.

Turned out, the branch that was missing from coverage can't happen with the current implementation (see comment for explanation).

[codecov]

Codecov Report

Merging #419 (e8acd1b) into trunk (6045131) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            trunk     #419      +/-   ##
==========================================
+ Coverage   96.73%   96.91%   +0.17%     
==========================================
  Files          13       13              
  Lines         552      551       -1     
  Branches      114      112       -2     
==========================================
  Hits          534      534              
  Misses          9        9              
+ Partials        9        8       -1     

Impacted Files	Coverage Δ
src/towncrier/_writer.py	100.00% <100.00%> (+4.34%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[hynek]

Please don't merge yet I've had an epiphany at the gym how simplify further :)

[hynek]

ok, it's ⚡️perfect⚡️ now – pls review 😇

[adiroiban]

Thanks for the update.

Once the windows test are green, this should be merged.

[hynek]

OK so here's the deal:

Writing it out without touching newlines seems to be the correct way of handling things. Otherwise some \r are converted. The open keyword for that is newlines="".

Since _builder.find_fragments doesn't convert newlines, because it loads using bytes, we get \r\n in the fragments coming into append_to_newsfile

However.

Reading an existing news file while leaving newlines in place breaks the detection of start_string whose default includes a \n: ".. towncrier release notes start\n"

If we read a file with \r\n it doesn't match, because the line becomes .. towncrier release notes start\r\n.

---

It's a total mystery to me how this worked until now. But I'll try to figure it out.

OK I get it. append_to_newsfile actually used unicode/universal newlines for reading, but not for writing. So fixing this is easy, I'm just not sure this whole thing makes a lot of sense.

[hynek]

OK, this is done as far as I'm concerned.

I'm deeply unhappy with what this refactoring has unearthed, but that's a topic for another day.

[adiroiban]

Thanks.

I think that is good that you found this bug.
And now we have better documentation/comments for towncrier's behaviour.

A few weeks ago I had no idea that single_file is a thing and that people might want to use it.

But, with GitHub Release automate release notes generation providing a simple alternative to towncrier, I guess that for towncrier to continue to stay relevant it shouod support all kind of different usage scenarios.

[adiroiban]

I guess we can leave this as it is. No preference.

[adiroiban]

Thanks, @hynek for the help with the coverage.

I am using something like this for codecov.yaml

coverage:
  precision: 2
  round: down
  range: 95..100
  status:
    patch:
      default:
        target: 100%
    project:
      default:
        target: auto
        # Temporary allow for a bit of slack in overall code coverage due to
        # swinging coverage that is not triggered by changes in a PR.
        threshold: 0.02%
      tests:
        target: 100%
        paths:
          - "src/towncrier/test/*"

It does 3 things:

• Make sure new code / modified code has at least 100% coverage :)
• Allows some slack to general project reporting, If one env (like macOS or Win) goes down we will see a bigger drop so this will get red
• Makes sure all the testing code will always have 100% - no point in keeping testing code that is not executed by the CI. This helps with the test code cleanup.