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

[5.10] Revert 'Add missing dependency (#1414)' #1483

Open
wants to merge 1 commit into
base: release/5.10
Choose a base branch
from

Conversation

finagolfin
Copy link
Contributor

@finagolfin finagolfin commented Nov 12, 2023

Cherrypick of #1481

Explanation: The llbuild executable is not a dependency of this library.

Scope: Removes wrongly added llbuild dependency

Issue: None

Risk: low, as less code is built after this

Testing: Passed all CI on trunk

Reviewer: @artemcm

The llbuild executable is not a dependency of this library.
Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

@artemcm
Copy link
Contributor

artemcm commented Nov 13, 2023

@swift-ci test

@finagolfin
Copy link
Contributor Author

Ping, another CI run and then this can go in?

@finagolfin
Copy link
Contributor Author

@neonichu, would be good to get this in before the winter break.

@finagolfin
Copy link
Contributor Author

@kateinoigakukun, please run the CI on this.

@kateinoigakukun
Copy link
Member

@swift-ci Please test

@finagolfin
Copy link
Contributor Author

Another CI run needed here.

@artemcm, looks like this is only held up on 5.10 manager approval, is there someone else who can review?

@finagolfin
Copy link
Contributor Author

@kateinoigakukun, one last CI run and we can get this in.

@kateinoigakukun
Copy link
Member

@swift-ci test

@finagolfin
Copy link
Contributor Author

Ping @nkcsgexi, ready for review and merge.

@finagolfin
Copy link
Contributor Author

Ping @nkcsgexi, still looking for review here and I figure you are the manager for this repo.

@finagolfin
Copy link
Contributor Author

Ping @artemcm, can we go ahead and get this in?

@finagolfin
Copy link
Contributor Author

@bnbarham, would you review this tiny pull for 5.10?

@finagolfin
Copy link
Contributor Author

@neonichu, this revert of your pull has been stuck for months now, can you review or alert whoever needs to decide?

@finagolfin
Copy link
Contributor Author

Pinging @bnbarham again, can we get this in now?

@finagolfin
Copy link
Contributor Author

Ping @DougGregor, there is no 5.10 release manager listed for this repo on the forum, but maybe it falls under your purview as part of the compiler.

This is just a small build cleanup that shouldn't break anything, been working fine in trunk for almost three months now.

@finagolfin
Copy link
Contributor Author

Ping @neonichu, can you notify someone who can get this in?

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.

None yet

3 participants