-
-
Notifications
You must be signed in to change notification settings - Fork 122
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
Remove LTO-only builds when PGO+LTO exists #223
Conversation
Unclear on the best way to test this (if any?). My methodology was to grep for |
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.
The important change here is to release.rs
, which selects which builds to release. Strictly speaking we could continue to build the LTO only configurations and just not release them. There are plenty of build configurations we build but don't release.
But, this project already suffers from too many CI jobs. More jobs, more chance of failure. I really don't think the LTO only builds provide much value on their own. So I'm fine removing them from CI as well.
As for the CI failure, could you please try rebasing onto latest main
commit? I think I fixed a bug in CI from forks.
Actually, I cherry-picked the hopeful CI fix commit to your branch. Let's see if CI works now. Feel free to rebase the branch at your leisure. The latest HEAD commit should disappear. |
@indygreg -- Thanks for cherry-picking. Want me to rebase or will you squash-merge either way? |
Please rebase. (I could do this myself but I try not to force push to other people's branches as a matter of courtesy since Git doesn't handle force pushes very well!) I also try to have a linear, merge-free history in the repo. Up to this point usually I've been cherry picking commits locally and amending commit messages in flight. It isn't very GitHub-y. But I don't need to be a stickler for this preference going forward. |
6268dce
to
9bfaba0
Compare
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.
Looks good!
Summary
This PR removes
lto
builds for cases in whichpgo+lto
exists, which looked like a good first task to familiarize myself with the project structure.Closes #220.