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 dead android-related code #4975

Merged
merged 1 commit into from
Aug 25, 2020
Merged

Remove dead android-related code #4975

merged 1 commit into from
Aug 25, 2020

Conversation

mossheim
Copy link
Contributor

Purpose and Motivation

remove android-related code for scsynth that hasn't been working since.. v3.4?

Types of changes

  • Cleanup

To-do list

  • Code is tested
  • All tests are passing
  • Updated documentation
  • This PR is ready for review

@dyfer
Copy link
Member

dyfer commented May 26, 2020

If someone decides to pick up on work to port SC to Android, would it be useful for them to be able to find this more easily than checking out an old SC branch? I guess my question is whether we'd want to leave this around for the time being.

@mossheim
Copy link
Contributor Author

If someone decides to pick up on work to port SC to Android, would it be useful for them to be able to find this more easily than checking out an old SC branch? I guess my question is whether we'd want to leave this around for the time being.

hmm, why is this hypothetical person who hasn't appeared in 8+ years capable of finding the code here, but not capable of looking in the changelog or git history, or asking project maintainers?

@mossheim
Copy link
Contributor Author

sorry, maybe a bad joke. what i mean is, i think it's okay to remove this because there are lots of other ways a person interested in doing that work would be able to find it.

@mossheim
Copy link
Contributor Author

mossheim commented Aug 9, 2020

@dyfer what do you think? i'd really like to remove this.

@elgiano
Copy link
Contributor

elgiano commented Aug 10, 2020

My 2c: this code is part of a project that has been inactive for more than 8 years. Everything changed in the meanwhile, and, following also some discussion on the forum, I think a modern implementation could be done with OpenSLES, and, in general the android code we have now is as good an example as SC_CoreAudio would be.
So, I would call this safe to remove, also because there's ongoing discussion about SC and Android on the forum here, so that anyone interested in working on this can see there's some interest already there.

Copy link
Contributor

@elgiano elgiano left a comment

Choose a reason for hiding this comment

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

lgtm! and I haven't found any other mention of android in the whole source code

@dyfer
Copy link
Member

dyfer commented Aug 10, 2020

thanks for your comments @brianlheim and @elgiano

I wasn't sure about this just because I didn't feel I have enough perspective. But from the conversation I see that there's no reason to keep this around, as any possible "current" Android implementation would look very different anyway.

Should I still merge this even though it's aimed at 3.11 and we're in the middle of the release?

@mossheim
Copy link
Contributor Author

Should I still merge this even though it's aimed at 3.11 and we're in the middle of the release?

good point, let's wait. 👍

thanks for the quick responses @elgiano and @dyfer. :)

@mossheim mossheim merged commit 49b3094 into supercollider:3.11 Aug 25, 2020
@mossheim mossheim deleted the topic/remove-dead-android-code branch August 25, 2020 12:01
@dyfer
Copy link
Member

dyfer commented Aug 30, 2020

Thanks! Just checking - as this went into 3.11: In case we don't release 3.11.2, would we need to remember to merge this (or 3.11 branch) into develop?

@mossheim
Copy link
Contributor Author

In case we don't release 3.11.2, would we need to remember to merge this (or 3.11 branch) into develop?

yeah, we'd merge 3.11 into develop one last time before starting a 3.12 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants