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.

Sign up for GitHub

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

Jump to bottom

Disable LTC again #1400

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Disable LTC again #1400

merged 1 commit into from
Sep 22, 2022

Conversation

silvasean
Copy link
Contributor

tanyokwok
tanyokwok approved these changes Sep 22, 2022
View reviewed changes
Copy link
Collaborator

@tanyokwok tanyokwok left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @silvasean

@dellis23
Copy link
Collaborator

Thanks!

@antoniojkim
Copy link
Collaborator

I would very highly prefer that we fix the issue instead of just disabling LTC again. I have a PR up to fix the issue here
#1401

@silvasean
Copy link
Contributor Author

Great! Thanks for the quick fix.

The general feeling in LLVM community development is to get HEAD back to a green state as fast as possible, rather than waiting for "fixing forward". See e.g. our patch reversion policy https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy
In this case, it's awesome that we were able to get a fix so quickly, however in my experience usually these things can take longer, so it's always "safe" to go back to green and generally preferable to get HEAD green again immediately.

@silvasean silvasean closed this Sep 22, 2022
@silvasean silvasean reopened this Sep 22, 2022
@silvasean silvasean closed this Sep 22, 2022
@ashay ashay reopened this Sep 22, 2022
@ashay
Copy link
Collaborator

ashay commented Sep 22, 2022

Merging this now to unblock other PRs.

@ashay ashay merged commit 566234f into llvm:main Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tanyokwok tanyokwok tanyokwok approved these changes

@dellis23 dellis23 dellis23 approved these changes

@antoniojkim antoniojkim Awaiting requested review from antoniojkim

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@silvasean @dellis23 @antoniojkim @ashay @tanyokwok