-
Notifications
You must be signed in to change notification settings - Fork 54
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
When Compiling serialize seperately per julia version #315
Conversation
oops looks like i fixed it on an old version, rebasing and retest now |
confirm still fixed |
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.
LGTM
Can we make this serialize based on major.minor? Or can serialization change in patch versions? |
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 feel like keeping things simple and just using the whole VERSION is better than trying to be clever. |
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.
Eventually we'll probably change the approach here to use scratch spaces: #295 (comment)
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.
Looks good. Just going to get CI working to ensure everything is working as expected here
Codecov Report
@@ Coverage Diff @@
## master #315 +/- ##
=======================================
Coverage 93.73% 93.73%
=======================================
Files 32 32
Lines 1532 1532
=======================================
Hits 1436 1436
Misses 96 96
Continue to review full report at Codecov.
|
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. |
Co-authored-by: Curtis Vogt <[email protected]>
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.