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

Show additional context for unrolled perf builds bisections #298

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

lqd
Copy link
Member

@lqd lqd commented Nov 1, 2023

In case the commit has been GC'ed on github, the user would need to map back from the commit to the PR by looking at the comment posted by the bot. This PR automates this, and displays the PR number and title (when available) in addition to the perf build commit.

In case we can't find any context, we panic and ask to open an issue. I'm not enamored with that but it's better in my mind than parsing and displaying an incorrect context (ensuring we have the same number of commits and descriptions).

This should only happen on incredibly rare events where the comment doesn't match either the v1 or v2 structure, and if that happens, the bisection would fail or its results would be bogus, so I think it's fine. (The other case is that there could be a bug in the parsing, which we could find only by it failing in the wild and people opening issues pointing at the unexpected perfbot comment).

This should show the PR number for v1, and the number and title for the current v2.

Fixes #294.

This is in case the commit has been GC'ed on github: the user would need to map
back from the commit to the PR by looking at the comment posted by the
bot. This can be automated.
@lqd lqd force-pushed the unrolled-contexts branch from a790a2e to cf0361b Compare November 1, 2023 14:14
Copy link
Collaborator

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! I'm not familiar with the different forms of comments the bot posts, but I trust you know what is needed.

@ehuss ehuss merged commit 9af234f into rust-lang:master Nov 1, 2023
@lqd
Copy link
Member Author

lqd commented Nov 1, 2023

I hope :) I’ll try to keep an eye on new issues in case some parsing error happens, don’t hesitate to ping me if I miss anything.
I had put real examples of the two formats in the unit tests last time I worked in this area, but it’s not super easy to have lasting reproducible tests: even matthias’ case in that issue has already passed the 160 day delay for artifacts. I couldn’t use it to ensure things worked as expected so I hope the tests are enough.

@lqd lqd deleted the unrolled-contexts branch November 1, 2023 22:50
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.

bisects to commits that do not exist
2 participants