Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential V4 nits/breaking changes? #1966

Closed
bartelink opened this issue Oct 10, 2023 · 2 comments
Closed

Potential V4 nits/breaking changes? #1966

bartelink opened this issue Oct 10, 2023 · 2 comments

Comments

@bartelink
Copy link
Member

bartelink commented Oct 10, 2023

Minor nits from perusing the source; most are likely breaking changes best kept for v4

  • TFMs
    • maybe a note somewhere about having a net6.0 TFM but explaining that no egregious later ones will be added see (see reasoning in Add net6.0 Target serilog-sinks-console#145 (comment))
    • remove net5.0 TFM (see above)
    • remove net7.0 TFM (atm it has identical DefineConstants values to net6.0)
    • Remove netcoreapp3.1, net5.0 from tests
  • README
    • Is there a stance/policy on copyright years (atm readme has 2020)

    • var log = new LoggerConfiguration() should gain a using

    • Log.CloseAndFlush(); in static example

      • should probably gain a // Normally in a try/finally; ensures File, Async etc sinks have been flushed before program exits`
      • or, more tersely '// ensure buffering or async Sinks are flushed before app exits'
    • move gitter mention further down the list? or remove?

    • We welcome bug reports and suggestions through our

      We welcome reproducable bug reports and detailed feature requests through our issue tracker here on GitHub; please note the other avenues are much better or for quick questions or seeking usage help

  • 🤔 remove changes.md given it stops dead at 2.10 (or sync with GH release notes)
  • AssemblyVersion [for 3.x] is 2.0, maybe shift to 4.0 in v4?
  • on https://github.com/serilog/serilog/wiki/Getting-Started, in the sample app
    • maybe lose and Windows Phone 8+. (maybe add some Blazor mention?)
    • use a top level namespace SerilogExample
    • consider using await Log.CloseAndFlushAsync()
    • (if this makes sense, ping me and I can do the edits)
  • 914M on https://www.nuget.org/packages/Serilog/ is insane (currently # 8 all time, should be 7 soon) - not long to go to 1B!

(Happy to do PRs for any that get agreed; equally happy for someone else to do them...)

@nblumhardt
Copy link
Member

These all look good, just dropping thoughts in here to try unblocking as many as possible.

TFMs

This one will need a standalone ticket for discussion, keen to simplify things where possible but also eager to avoid covering all the ground covered in the last thread over again :-)

Is there a stance/policy on copyright years (atm readme has 2020)

I think we've moved away from specifying the year, "Copyright © Serilog Contributors" is preferred

move gitter mention further down the list? or remove?

Remove 👍

🤔 remove changes.md given it stops dead at 2.10 (or sync with GH release notes)

Remove 👍

AssemblyVersion [for 3.x] is 2.0, maybe shift to 4.0 in v4?

May pay to search past issues; I think the intention was to freeze it for all time (completely kill off all binding redirect related support) but tooling has moved on so kicking it up to 4.0 might end up being better now.

maybe lose and Windows Phone 8+. (maybe add some Blazor mention?)

👍

use a top level namespace SerilogExample

May just be noise, it's not strictly required AFAIK.

consider using await Log.CloseAndFlushAsync()

Definitely 👍

@bartelink
Copy link
Member Author

TFMs

This one will need a standalone ticket for discussion, keen to simplify things where possible but also eager to avoid covering all the ground covered in the last thread over again :-)

OK, will do

Seems I was looking at an old version and/or you resolved all of them, thanks!

Did a wiki edit

My remaining items are addressed in #1969

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants