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

Bound Memento 1.x releases to pre-1.3 #41

Merged
merged 3 commits into from
Nov 17, 2021
Merged

Conversation

rofinn
Copy link
Member

@rofinn rofinn commented Nov 10, 2021

Previously, Memento would store UTC ZonedDateTimes in the records and explicitly depended on TimeZones.jl. In 1.3, it switched to only storing DateTime in order to make TimeZones.jl an optional dependency. Since the rest of the Julia ecosystem defaults to localzone rather than UTC it made sense to update both packages to follow that same conventions.

invenia/Memento.jl#182 (comment)

Previously, Memento would store UTC ZonedDateTimes in the records and explicitly depended on TimeZones.jl. In 1.3, it switched to only storing `DateTime` in order to make TimeZones.jl an optional dependency. Since the rest of the Julia ecosystem defaults to `localzone` rather than UTC it made sense to update both packages to follow that same conventions.

invenia/Memento.jl#182 (comment)
@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #41 (2690eb6) into master (7f95ed1) will decrease coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   94.35%   94.21%   -0.14%     
==========================================
  Files           5        5              
  Lines         177      173       -4     
==========================================
- Hits          167      163       -4     
  Misses         10       10              
Impacted Files Coverage Δ
src/handler.jl 92.68% <0.00%> (-0.35%) ⬇️
src/stream.jl 93.33% <0.00%> (-0.07%) ⬇️
src/exceptions.jl 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f95ed1...2690eb6. Read the comment docs.

@iamed2
Copy link
Member

iamed2 commented Nov 10, 2021

This does not actually change the version compatibility at all. See https://pkgdocs.julialang.org/v1/compatibility/#Version-specifier-format

You want this: ~1.0, ~1.1, ~1.2

Project.toml Outdated Show resolved Hide resolved
Co-authored-by: Eric Davies <iamed2@gmail.com>
@iamed2
Copy link
Member

iamed2 commented Nov 10, 2021

@rofinn Will you handle the manual General registry edit or do I need to do that?

@rofinn
Copy link
Member Author

rofinn commented Nov 10, 2021

manual General registry edit or do I need to do that?

Wait, what? So we need to retroactively upper bound every passed release manually? This is such a bad system for such a common problem.

@iamed2
Copy link
Member

iamed2 commented Nov 10, 2021

manual General registry edit or do I need to do that?

Wait, what? So we need to retroactively upper bound every passed release manually? This is such a bad system for such a common problem.

Why should releasing a version 2 with some compat affect the compat for version 1?

You don't need to make a change for every version, it's a 2-line change in https://github.com/JuliaRegistries/General/blob/master/C/CloudWatchLogs/Compat.toml since Memento v1+ is only mentioned twice.

@rofinn
Copy link
Member Author

rofinn commented Nov 10, 2021

No, I understand why adding an upperbound to v2 won't automatically update v1. My issue is with the fact that the fix is to manually edit some registry file.

@rofinn
Copy link
Member Author

rofinn commented Nov 10, 2021

Looks like I was looking for something like Tim Holy's propagate_upperbounds function.

@@ -14,7 +14,7 @@ UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
[compat]
AWS = "1"
MbedTLS = "1"
Memento = "1"
Memento = "~1.0, ~1.1, ~1.2"
Copy link
Member Author

Choose a reason for hiding this comment

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

Could this be simplified to?

Suggested change
Memento = "~1.0, ~1.1, ~1.2"
Memento = "1 - 1.2"

Copy link
Member

Choose a reason for hiding this comment

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

No, since hyphens require Julia 1.4

Project.toml Outdated Show resolved Hide resolved
@rofinn
Copy link
Member Author

rofinn commented Nov 10, 2021

Since the registry update has been merged I'm just gonna include the Memento compat change for now.
@rofinn
Copy link
Member Author

rofinn commented Nov 12, 2021

Okay, the registry change was merged, so I think we're good to just merge the memento compat in until I update to the new memento release.

@rofinn
Copy link
Member Author

rofinn commented Nov 15, 2021

Bump. Is this good to merge?

@iamed2
Copy link
Member

iamed2 commented Nov 17, 2021

Sorry, missed your comment from 4 days ago, yes let's merge

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