-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
Add commit information to roc versions built from source #7046
base: main
Are you sure you want to change the base?
Conversation
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.
Just left some comments explaining my changes
Nice, this is something I considered but wasn't sure about, so I just used a package for the initial version, I'll switch to using |
6d0a9b6
to
db485ad
Compare
@Anton-4 I've removed
and when the repo has changes:
Let me know if you think there's anything that I can improve :) |
be27201
to
f52697f
Compare
a634d67
to
82256b6
Compare
Thanks for your comments @Anton-4, I've addressed them all now 👍 |
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 will be really useful. Thank you @Hasnep
To fix the test failures you'll need to do |
13c3f6a
to
80aaf0c
Compare
The |
Yes, that is true 😅 I think instead of panicking with "Failed to parse timestamp as an integer" we could default to the string "missing timestamp", that would be most inline with the nix philosophy vs trying to work around this. |
I've added a check to see if the |
d672180
to
4170f47
Compare
4170f47
to
058ef10
Compare
Addresses #7030.
I'm not that experienced with Rust and I'm not familiar with the style and priorities of the Roc compiler team, so I'd appreciate feedback on anything that looks out of place or inefficient 😅
I think this is nearly done, but I'm getting a default commit timestamp when I run it locally:
I think it's a default timestamp, maybe for reproducible builds? But that doesn't make sense to me because it's not a build timestamp, it's a commit timestamp 🤔
Again, any help is appreciated! 😀