-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Iterating on human-readable AMP version numbers #27972
Comments
Makes sense to me! |
Slightly related: ampproject/error-tracker#125 I support this, especially because of the added benefit that developers can then look at the date embedded in an RTV and, with some confidence, say that their PR is/isn't in it based on the datetime their commit merged |
This is already how it's done since #16631. It takes the commit time, which is different from author time - basically it's the time somebody clicked the "squash" button on GitHub. Before #27848, cherry-picks would cause that commit time to shift to when the release-on-duty ran |
Oh yea!! 🎉 |
Can we convert to Eastern or Pacific Time? Seems like it's in UTC. For example, I created a cherry pick pick today, April 23, between 7:30 and 8:30 ET. The cherry pick instructions gave me a |
UTC was a deliberate choice as it is never affected by DST. It's a 1:1,000,000,000 chance that someone will build two releases one after the in the one hour span during which DST ends, but why take the risk :P |
Follow-up to #27793.
Can we consider changing the computation of the RTV from "time of release cut" to "time of most top commit"? This would be a nice property for human-readability.
I'm guessing this was originally decided against since cherry-picks can cause multiple releases to have the same RTV, but now cherry-picks onto a release preserves most of the base RTV so maybe we're good to go?
Context is I was recently bisecting releases to pinpoint a regression and I couldn't rely on the dates baked into the RTVs to determine ordering e.g. RTV 123 is not necessarily a subset of RTV 124. So I had to look up each nightly's commit SHA to figure out where it was relative to the stable RTV.
/cc @ampproject/wg-infra
The text was updated successfully, but these errors were encountered: