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

Remove unused Apple simulator starlark API #19169

Conversation

keith
Copy link
Member

@keith keith commented Aug 3, 2023

As far as I can tell these starlark APIs have never been used by rules_apple, and I can't find any other uses of them on GitHub. The intent of these was to replace the iOS specific API which is used in rules_apple but at this point if these are ever needed by the rules we should use a custom starlark flag instead of a bazel flag.

@keith keith requested a review from lberki as a code owner August 3, 2023 23:06
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Aug 3, 2023
As far as I can tell these starlark APIs have never been used by
rules_apple, and I can't find any other uses of them on GitHub. The
intent of these was to replace the iOS specific API which is used in
rules_apple but at this point if these are ever needed by the rules we
should use a custom starlark flag instead of a bazel flag.
@keith keith force-pushed the ks/remove-unused-apple-simulator-starlark-api branch from 2a029f4 to 4149cf6 Compare August 3, 2023 23:07
@keith
Copy link
Member Author

keith commented Aug 3, 2023

maybe google uses these internally? (not on the upstream branch of rules_apple but somewhere else)

@sgowroji sgowroji added the team-Rules-ObjC Issues for Objective-C maintainers label Aug 4, 2023
@nglevin
Copy link
Contributor

nglevin commented Aug 8, 2023

These do indeed look to be dead code. I'll have a closer look by patching Bazel derivatives to see if anything at all changes with these removed.

If nothing at all changes, I'm happy to sign off on this one.

@nglevin
Copy link
Contributor

nglevin commented Aug 9, 2023

It appears that there's no visible changes in the interactions with internal bits. This particular change seems safe to me.

Will likely sign off after the buildkite checks complete, as long as they look good.

@nglevin nglevin added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Aug 9, 2023
@copybara-service copybara-service bot closed this in e6d7c16 Aug 9, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-ObjC Issues for Objective-C maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants