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

Update changelog #1246

Closed
wants to merge 5 commits into from
Closed

Update changelog #1246

wants to merge 5 commits into from

Conversation

pitdicker
Copy link
Collaborator

Mostly because I like having a CHANGELOG.md file, and don't mind doing some simple busywork.

I included old releases that were skipped in the changelog but had release notes in the git commit of the version bump, and the newer releases that only had GitHub releases.

I also did some spelunking yesterday. Is it nice to include the history of the relation between chrono and time 0.1?
Including release notes in the changelog is a little out of place, but not too much I hope.

@codecov
Copy link

codecov bot commented Sep 2, 2023

Codecov Report

Merging #1246 (ed4a657) into 0.4.x (77d0bec) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            0.4.x    #1246   +/-   ##
=======================================
  Coverage   91.40%   91.40%           
=======================================
  Files          38       38           
  Lines       16932    16932           
=======================================
  Hits        15477    15477           
  Misses       1455     1455           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pitdicker pitdicker force-pushed the changelog branch 7 times, most recently from 8e19c02 to a9fc4fe Compare September 4, 2023 11:36
@pitdicker
Copy link
Collaborator Author

I have tried to write an accurate description of the issue in RUSTSEC-2020-0159:

The issue: on Unix-like systems a process is given a time zone id or description via the TZ environment variable. We need this time zone data to calculate the current local time from a value that is in UTC, such as the time from the system clock. time 0.1 and chrono used the POSIX function localtime_r to do the conversion to local time, which reads the TZ variable.

Rust assumes the environment to be writable and uses locks to access it from multiple threads. So do some other programming languages and libraries, but there is no shared locking mechanism. More importantly POSIX declares modifying the environment in a multi-threaded process as unsafe, and getenv in libc can't be changed to just take a lock because it returns a pointer to the data (see rust#27970 for more discussion).

Since chrono#677, released as version 4.20, chrono no longer uses localtime_r but provides a pure-rust solution. This makes reading the TZ variable if changed by Rust code sound.

@pitdicker pitdicker force-pushed the changelog branch 3 times, most recently from d4d6df9 to 4357724 Compare September 4, 2023 20:15
@pitdicker
Copy link
Collaborator Author

I have collected past release notes in a RELEASE_NOTES.md file. Let me know what you think 😄.

@djc
Copy link
Member

djc commented Sep 5, 2023

Mostly because I like having a CHANGELOG.md file, and don't mind doing some simple busywork.

Sorry, I don't think this a good enough reason. This means I have to review your "busywork", generating busywork for me which I do mind. There's also the issue of having the same content in two places which can be confusing for users.

If you want to backfill GitHub releases, that sounds great! If you want to collect some more history in the documentation, that would also be great, but I don't think the release notes are a great place for it.

@pitdicker
Copy link
Collaborator Author

If you want to collect some more history in the documentation, that would also be great, but I don't think the release notes are a great place for it.

That part I agree with. Can you suggest a better place?

@pitdicker
Copy link
Collaborator Author

If you want to backfill GitHub releases, that sounds great!

I have added GitHub releases for all releases on crates.io. For one I had to add a missing git tag.

@djc
Copy link
Member

djc commented Sep 6, 2023

If you want to collect some more history in the documentation, that would also be great, but I don't think the release notes are a great place for it.

That part I agree with. Can you suggest a better place?

I'd just make it a section in the crate root documentation, probably somewhere near the bottom.

@pitdicker
Copy link
Collaborator Author

pitdicker commented Sep 6, 2023

Sorry, I don't really follow what you have in mind. The stuff I added to that file is currently ca. 800 lines.
You want to add that to lib.rs? Or only the last part?

I would like to keep the other history somewhere also. But it can be a copy on my hard drive 😄.

@djc
Copy link
Member

djc commented Sep 6, 2023

Ah, sorry -- I would splice the release notes into the GitHub Releases similar to what we've done recently, except for the stuff in Unreleased which I would append to the end of our crate root documentation.

@pitdicker
Copy link
Collaborator Author

I have added it to the main documentation and updated the following GitHub Releases:
https://github.com/chronotope/chrono/releases/tag/v0.4.0
https://github.com/chronotope/chrono/releases/tag/v0.3.0
https://github.com/chronotope/chrono/releases/tag/v0.2.0
https://github.com/chronotope/chrono/releases/tag/v0.1.0

Is it okay to take the additions / fixes to the changelog that I took from the git commits?

@djc
Copy link
Member

djc commented Sep 11, 2023

So this can be closed, right?

@pitdicker
Copy link
Collaborator Author

I hope it is ok with you to merge these changes.
It are additions and some formatting fixes to make the changelog pre-0.4.20 complete. With those it would be in sync with the descriptions in the GitHub Releases.

@djc
Copy link
Member

djc commented Sep 22, 2023

As mentioned,

Sorry, I don't think this a good enough reason. This means I have to review your "busywork", generating busywork for me which I do mind. There's also the issue of having the same content in two places which can be confusing for users.

If you want to backfill GitHub releases, that sounds great! If you want to collect some more history in the documentation, that would also be great, but I don't think the release notes are a great place for it.

I don't want to bring back CHANGELOG.md.

@pitdicker pitdicker closed this Sep 22, 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.

2 participants