Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork847
Use UTC for future release date#926
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Use UTC for future release date#926
Uh oh!
There was an error while loading.Please reload this page.
Conversation
9b617c2 to1139c1bCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks a lot, this looks like a quality of life change!
| end | ||
| it { is_expected.to eq(["2.0.0", "2.0.0", Time.gm(2020, 12, 28, 3)]) } | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for adding a spec, this helps catch regressions. But, perhaps a code comment, or an "it" being described with a note that captures the essence of the change (should always use UTC since GitHub uses UTC for the things we care about, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
lol, I was focusing what was being tested and the reason for this has been completely lost!
I amended my commit to add a context and an explicit comment. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This looks perfect, and will guide the interested reader in the future.
Thanks,@smortex for the repairs!
1139c1b to0ed7f4cCompareWhen using future releases, the current local date is writen in thegenerated CHANGELOG. This is usualy done as part of releasing a newversion, that is a few seconds before a that version is tagged andthat tag is pushed to GitHub.On next version bump, that previous tag exist and it's date is read fromGitHub which provide this information in Coordinated Universal Time(UTC).Depending on the time-zone of the user, the date of these two Ruby Timeobjects may be different, leading to an unexpected change in theprevious entry of the CHANGELOG.To avoid this, convert the local time to UTC when we do not have anactual tag time to use.
0ed7f4c to2a44a8fCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM!
| end | ||
| it { is_expected.to eq(["2.0.0", "2.0.0", Time.gm(2020, 12, 28, 3)]) } | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This looks perfect, and will guide the interested reader in the future.
Thanks,@smortex for the repairs!
When using future releases, the current local date is writen in the
generated CHANGELOG. This is usualy done as part of releasing a new
version, that is a few seconds before a that version is tagged and
that tag is pushed to GitHub.
On next version bump, that previous tag exist and it's date is read from
GitHub which provide this information in Coordinated Universal Time
(UTC).
Depending on the time-zone of the user, the date of these two Ruby Time
objects may be different, leading to an unexpected change in the
previous entry of the CHANGELOG.
To avoid this, convert the local time to UTC when we do not have an
actual tag time to use.