-
Notifications
You must be signed in to change notification settings - Fork 778
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
common: Rename shardingForkDev hf to Cancun #2659
Conversation
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
{ | ||
"name": "cancun", | ||
"block": null, | ||
"forkHash": null |
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.
I wonder if me might always want to take the sematics here:
If there is an entry on something with null
, there is (will be) some value, we just don't know (yet) what it is, and completely leaving when there is just not a value.
This would be an argument to also add "timestamp": null
for these HF entries (while having (obviously) left the timestamp
entry for a HF like London).
I have absolutely no idea/overview, if this has practical implications or not, but it might help that we can do some things slightly cleaner already or in the future.
Totally no strong opinion here though, might help a bit, might be a total "non-thing", feel free to add or leave (if you e.g. do not want to re-run CI).
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.
timestamp is currently undefined or string i think, can change/add null in followup PR 👍
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.
I can already approve here independently from the one thing I mentioned.
If you want to still add I can directly re-approve, otherwise just merge in.
With Cancun hf shaping up nicely, this PR updates the references of the hf to
Cancun
(corresponding CL hfDeneb
)