search

JuliaTime / TimeZones.jl

IANA time zone database access for the Julia programming language
Other
86 stars 52 forks source link

When Compiling serialize seperately per julia version #315

Closed oxinabox closed 3 years ago

oxinabox commented 3 years ago

Fixes #288 (and its duplicate #309).

I have tested locally that this works to fix that problem. It's hard to test it in CI since it requires starting multiple julia instances.

oxinabox commented 3 years ago

oops looks like i fixed it on an old version, rebasing and retest now

oxinabox commented 3 years ago

confirm still fixed

iamed2 commented 3 years ago

Can we make this serialize based on major.minor? Or can serialization change in patch versions?

oxinabox commented 3 years ago

It probably can change based on patch versions if there is bug in how serialisation is done that needs changed to the format to fix, which is then backported. (I know I have seen some bugs which resulted in such changes idk if backported though) More importantly if you are using Julia nightly/source build it can definitely change based on the pre-release build suffix.

I feel like keeping things simple and just using the whole VERSION is better than trying to be clever. It makes it marginally slower to install and uses a tiny bit more harddrive.

codecov-io commented 3 years ago

Codecov Report

Merging #315 (fb852db) into master (a314759) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #315   +/-   ##
=======================================
  Coverage   93.73%   93.73%           
=======================================
  Files          32       32           
  Lines        1532     1532           
=======================================
  Hits         1436     1436           
  Misses         96       96
Impacted Files Coverage Δ
src/tzdata/build.jl 63.63% <100.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 a314759...fb852db. Read the comment docs.

omus commented 3 years ago

I'm quite sure the benchmark failure is due to the compiled files being in different directories. As a build isn't triggered when running against the old commit then the compiled files are missing according to the base version. As I'm not expecting performance issues from these changes I'll go ahead and merge this.