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

Everywhere: Gently remove the ladybird android port #24555

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

nico
Copy link
Contributor

@nico nico commented Jun 9, 2024

With Ladybird now being its own repository, there's little reason to keep the Ladybird Android port in the SerenityOS repository.

(The Qt port is useful to be able to test changes to LibWeb in lagom so it'll stay around. Similar for the AppKit port, since getting Qt on macOS is a bit annoying. But if the AppKit port is too much pain to keep working, we should toss that too.

Eventually, the lagom browser ports should move out from Ladybird/ to Meta/Lagom/Contrib, but for now it might make sense to leave them where they are to keep cherry-picks from ladybird easier.)

With Ladybird now being its own repository, there's little reason
to keep the Ladybird Android port in the SerenityOS repository.

(The Qt port is useful to be able to test changes to LibWeb in lagom
so it'll stay around. Similar for the AppKit port, since getting
Qt on macOS is a bit annoying. But if the AppKit port is too much
pain to keep working, we should toss that too.

Eventually, the lagom browser ports should move out from Ladybird/
to Meta/Lagom/Contrib, but for now it might make sense to leave them
where they are to keep cherry-picks from ladybird easier.)
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 9, 2024
@kennethmyhra kennethmyhra added ✅ pr-community-approved PR has been approved by a community member and removed 👀 pr-needs-review PR needs review from a maintainer or community member labels Jun 10, 2024
@ADKaster
Copy link
Member

ADKaster commented Jun 10, 2024

Didn't catch it in my scroll through but there's a nightly android GitHub actions job that should be removed if you missed it

@ADKaster
Copy link
Member

Also, is removing all android support from the core libraries the right call? I've seen people run the Shell and js from inside termux before. which is neat, but also makes sense if we don't actually care :)

@nico
Copy link
Contributor Author

nico commented Jun 10, 2024

Also, is removing all android support from the core libraries the right call? I've seen people run the Shell and js from inside termux before. which is neat, but also makes sense if we don't actually care :)

I think it's fine to put that back if people actually run lagom binaries on Android, but I figured it's better to err on removing too much than too little.

@github-actions github-actions bot added 👀 pr-needs-review PR needs review from a maintainer or community member and removed ✅ pr-community-approved PR has been approved by a community member labels Jun 10, 2024
@nico
Copy link
Contributor Author

nico commented Jun 10, 2024

Didn't catch it in my scroll through but there's a nightly android GitHub actions job that should be removed if you missed it

Do I just delete that file? I added a commit for that, thanks. Could you take a look if it looks right?

@ADKaster
Copy link
Member

yes, just deleting the file is the way. for Azure DevOps there was a 'main' file that triggered the jobs, but for github just having the workflow file in the right place is the thing to do. looks good to me

@nico nico added ✅ pr-maintainer-approved-but-awaiting-ci PR has been approved by a maintainer and can be merged after CI has passed and removed 👀 pr-needs-review PR needs review from a maintainer or community member labels Jun 10, 2024
@github-actions github-actions bot added 👀 pr-needs-review PR needs review from a maintainer or community member and removed ✅ pr-maintainer-approved-but-awaiting-ci PR has been approved by a maintainer and can be merged after CI has passed labels Jun 11, 2024
@nico nico merged commit 3780499 into SerenityOS:master Jun 11, 2024
11 checks passed
@nico nico deleted the notdroid branch June 11, 2024 23:40
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 11, 2024
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.

3 participants