Skip to content
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

try builds: include a copyable version of the full commit SHA in comment #53

Merged
merged 1 commit into from
Aug 6, 2019

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jul 2, 2019

I regularly trigger try builds to then do a perf run. Perfbot needs the full 40-char SHA1 of the commit. To get that from what bors gives me on a successful try build (like this comment), I need to copy the URL of that link and then remove the junk before the commit.

With this, my hope is I will get a 40-char SHA1 that I can just double-click, Ctrl-C, done. :)
I don't know how to test this change, though.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Jul 2, 2019

This will work conceptually I think -- I should just fix rust-timer to not need that though.

Testing: d0297435b58c0279a1cdb36a5d80ca4ede4ff42a

@bryanburgers
Copy link
Contributor

I agree. I would expect your change to work. I’m not in a position where I can test easily the rest of the week though (4th of July holiday).

LGTM 👍🏼

@RalfJung
Copy link
Member Author

RalfJung commented Jul 2, 2019

This will work conceptually I think -- I should just fix rust-timer to not need that though.

Even then, a non-link text is easier to copy-paste.

@shepmaster
Copy link
Member

or add a link to perfbot? no copy-paste at all

@RalfJung
Copy link
Member Author

RalfJung commented Jul 4, 2019

What do you mean?

What would be nice is something like@bors try-for-perf or so that automatically triggers perfbot in the "try build successful" message. Bots talking to each other. Skynet is coming :)

@RalfJung
Copy link
Member Author

RalfJung commented Aug 6, 2019

More automation would be nice, but until then, just landing this would already help.

@Mark-Simulacrum
Copy link
Member

Just as a note, perf now supports shorter commit SHAs on input. rust-lang/rustc-perf@140a567

@RalfJung
Copy link
Member Author

RalfJung commented Aug 6, 2019

@Mark-Simulacrum that's great! I'd have to see how this works in practice; copy-pasting link still requires very careful selection to not also copy surrounding spaces and also not click the link.

So, fine for me to close this until we have further data -- though I also still think this change does make sense.

@Mark-Simulacrum
Copy link
Member

I think merging makes sense. We can always revert at a later point if desired.

@Mark-Simulacrum Mark-Simulacrum merged commit 91da2d8 into rust-lang:master Aug 6, 2019
@RalfJung
Copy link
Member Author

So either this did not work or it did not get deployed or so? rust-lang/rust#64600 (comment) does not contain a copyable commit.

But also it seems we have a new magic command that makes copying unnecessary: @bors try @rust-timer queue

@Mark-Simulacrum
Copy link
Member

We may not have re-deployed RCS since then since it usually leads to queue problems (so I'm hesitant to do it for no particular reason); let's go with the new magical command for now.

@RalfJung RalfJung deleted the try branch August 27, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants