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

chore(docs): Markdown housekeeping #1969

Merged
merged 7 commits into from Oct 26, 2023
Merged

Conversation

bartelink
Copy link
Member

@bartelink bartelink commented Oct 21, 2023

related: #1966

  • Remove year (2020) from copyright message on readme (note this propagates to nuget)
  • Remove mention of gitter (it's not well-monitored, so better to have people use GitHub issues and/or PRs to to/fro ideas)
  • Added NuGet package copyright (see far bottom right of https://www.nuget.org/packages/equinox)

Open issues:

  • Some files are missing copyright headers. Many have incomplete years.

    I feel the best answer is to remove them in favor of having one copyright year range in the NOTICE. This seems to be valid based on https://www.apache.org/legal/apply-license.html#copy-per-file and general SO searching. If this is not agreed, perhaps I could remove them from the test files only?

    Failing that, I can add the header to all source files.

It's likely to be frustrating as nobody actively monitors it (PRs/issues will do the job given current traffic)
README.md Outdated Show resolved Hide resolved
@bartelink bartelink marked this pull request as ready for review October 21, 2023 17:07
@@ -7,10 +7,11 @@
<CheckEolTargetFramework>false</CheckEolTargetFramework>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<Copyright>Copyright © 2013-23 Serilog Contributors</Copyright>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flows up to the nuget landing page/ package metadata (and I think the FILEVERSIONINFO on the DLL)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm the Authors in the package (i.e., <Authors>Serilog Contributors</Authors>) might get appended in some renderings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README.md Show resolved Hide resolved
@bartelink
Copy link
Member Author

Thoughts on the open issue (in the overview) ? (i.e. add missing copyright headers in impl/test files and/or a copyright to the LICENSE and/or adjust the years per file) ?

@nblumhardt
Copy link
Member

RE the copyright headers, I think sorting out the year range (© in the source files where they appear, and full range in the LICENSE file) would be my preference; no super strong feelings there though.

@bartelink
Copy link
Member Author

Not sure what sorting out the year range would mean; make them consistent and then update them every year?
Ensure they reflect the time of last edit per file?
Or strip off the date in the headers?

Happy to do anything of the following that you want
0. just leave it

  1. add copyright and year range in LICENSE
  2. remove years in file headers, leaving it as Copyright © Serilog Contributors or maybe Copyright 2013- ...
  3. add headers to all source files missing them
  4. add headers to all test files missing them
  5. remove all headers from source files

@bartelink
Copy link
Member Author

bartelink commented Oct 26, 2023

aside/unrelated: @nblumhardt can you have a peep at File sink PR 258 please?
Happy to continue reviewing until xmldoc aligns with impl, but perhaps it should just be called off as the motivation/need seems dubious at this point

@nblumhardt
Copy link
Member

Thanks for the follow-up. I think even just (2) - using Copyright © Serilog Contributors, and (3) add them consistently to all source files would do the trick, if that sounds good to you?

@bartelink
Copy link
Member Author

@nblumhardt thanks for the response - will merge this and do it as a fresh PR another time

One final question: do I remove headers from test files (a number 6 option I didnt propose!), or apply them consistently same as for source files (i.e. 4) ?

@nblumhardt
Copy link
Member

I'm fine either way on the test files - they don't tend to "travel" like the source files do (e.g. Azure/serilog-sinks-azuredataexplorer#16).

@bartelink
Copy link
Member Author

Hm, having seen that I'll apply them to the test files too, unless that becomes a truly huge change..

@bartelink bartelink merged commit e37837e into serilog:dev Oct 26, 2023
1 check passed
@bartelink bartelink deleted the bartelink/tidy branch October 26, 2023 23:45
@nblumhardt nblumhardt mentioned this pull request Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants