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

[libc++] Test GitHub PRs for libc++ patches #63273

Closed
wants to merge 1 commit into from

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jun 12, 2023

No description provided.

@xgupta xgupta self-requested a review June 25, 2023 16:29
@greg7mdp
Copy link

Can I submit a libc++ PR via github?

Copy link
Member

@poyaoc97 poyaoc97 left a comment

Choose a reason for hiding this comment

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

LGTM

@ldionne
Copy link
Member Author

ldionne commented Aug 23, 2023

Can I submit a libc++ PR via github?

Not yet, we're working on that!

I'm going to lock this issue as I pick this up again and start experimenting with the GH action configuration necessary to make this work. I suggest folks unsubscribe from this PR because this will get quite spammy :)

@llvm llvm locked and limited conversation to collaborators Aug 23, 2023
@ldionne ldionne force-pushed the ldionne-z-github-prs branch 2 times, most recently from 145df9b to 7f8cde6 Compare August 23, 2023 21:13
@ldionne
Copy link
Member Author

ldionne commented Aug 23, 2023

I am waiting for an admin to add the BuildKite API token I provided to the repo :-)

@ldionne ldionne force-pushed the ldionne-z-github-prs branch 18 times, most recently from 6101137 to c498911 Compare August 24, 2023 16:55
@ldionne ldionne force-pushed the ldionne-z-github-prs branch 20 times, most recently from 86f2481 to e58f9fc Compare August 29, 2023 13:51
This basically inlines the logic that was previously located in
https://github.com/google/llvm-premerge-checks so it is part of
the monorepo. This has the benefit of making it extremely easy
for individual projects to understand and modify this logic for
their own needs, unlike the current model where this logic lives
in a separate non-LLVM repository.

It also allows testing changes to the CI configuration using a simple
Phabricator review, since the code that defines the CI pipeline is taken
from the patch under review.

This (or something equivalent) is necessary if we want to retain the
current monolithic pre-commit CI throughout the GitHub PR transition.
Since triggering the monolithic CI is currently attached to the system
we use for triggering CI pipelines from Phabricator, we will lose that
part of the CI when we move to GitHub PRs if we don't do anything.

I've decided to rewrite the code as a shell script because the logic
was fairly simple and it seemed a lot easier than figuring out how to
pull only the relevant parts of llvm-premerge-checks into the monorepo.
Furthermore, I think we should strive to move away from the monolithic
CI altogether since sub-projects should understand, own and maintain the
tests that are relevant for them to run in the CI (with LLVM providing
the infrastructure). Hence, this is somewhat of a temporary solution
until monolithic CI is removed entirely.

Differential Revision: https://reviews.llvm.org/D158863
@ldionne
Copy link
Member Author

ldionne commented Aug 29, 2023

Closing this since cf1a3d9 was merged via Phabricator.

CI should work now for PRs but we'll probably need to make some tweaks.

@ldionne ldionne closed this Aug 29, 2023
@ldionne ldionne deleted the ldionne-z-github-prs branch August 29, 2023 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants