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

move RecipesBase support to an extension on 1.9+ #446

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

palday
Copy link
Contributor

@palday palday commented Aug 22, 2023

This uses the method in the Pkg docs to continue support for Julia < 1.9.

On 1.9, this shaves about 100ms off of using TimeZones for me.

@palday
Copy link
Contributor Author

palday commented Aug 22, 2023

For the Benchmark CI -- it looks like the environment isn't re-resolved when comparing to origin/HEAD and this causes errors. xref JuliaCI/PkgBenchmark.jl#137

src/TimeZones.jl Show resolved Hide resolved
ext/TimeZonesRecipesBaseExt.jl Outdated Show resolved Hide resolved
ext/TimeZonesRecipesBaseExt.jl Show resolved Hide resolved
Project.toml Show resolved Hide resolved
Comment on lines 32 to +33
[extras]
RecipesBase = "3cdcf5f2-1ef4-517c-9805-6587b60abb01"
Copy link
Member

Choose a reason for hiding this comment

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

This is now required as on Julia 1.9 the [weakdeps] take precedence over [deps] so in order to test with this we need to now add [extras] right?

That explains the PkgBenchmark failure better too. You could work around that failure by setting the Julia version for the benchmark CI tests to 1.8 so that the [weakdeps] are ignored and [deps] are preferred. I'm fine with ignoring the benchmark tests for this PR so don't feel the need to go through this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now required as on Julia 1.9 the [weakdeps] take precedence over [deps] so in order to test with this we need to now add [extras] right?

Correct.

I would also prefer to just ignore the failure for this one PR so that benchmarking remains on the current release moving forward.

Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Merging #446 (f9fb85a) into master (77c9c04) will increase coverage by 0.05%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
+ Coverage   92.66%   92.72%   +0.05%     
==========================================
  Files          40       40              
  Lines        1841     1841              
==========================================
+ Hits         1706     1707       +1     
+ Misses        135      134       -1     
Files Changed Coverage Δ
ext/TimeZonesRecipesBaseExt.jl 88.88% <ø> (ø)
src/TimeZones.jl 92.85% <ø> (ø)

... and 1 file with indirect coverage changes

@palday palday requested a review from omus August 23, 2023 17:00
@omus omus merged commit f9d0dcf into JuliaTime:master Aug 24, 2023
13 of 15 checks passed
@palday palday deleted the pa/recipesext branch August 25, 2023 01:48
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

3 participants