Skip to content

Comments

COMPILING.md - android instructions#1805

Merged
gilgongo merged 5 commits intojamulussoftware:masterfrom
ngocdh:patch-1
Jun 2, 2021
Merged

COMPILING.md - android instructions#1805
gilgongo merged 5 commits intojamulussoftware:masterfrom
ngocdh:patch-1

Conversation

@ngocdh
Copy link
Contributor

@ngocdh ngocdh commented Jun 1, 2021

No description provided.

@ngocdh ngocdh mentioned this pull request Jun 1, 2021
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Thank you!

@hoffie hoffie requested a review from gilgongo June 1, 2021 11:55
* Open the generated .xcodeproject in XCode, check the Signing & Capabilities and configure a team
* To run on a iOS device, build and run on the device, you'll have first trust your developer profile in device's Settings.

## Android
Copy link
Member

Choose a reason for hiding this comment

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

I don't know anything about Android development, but to refer explicitly to UI elements "the Projects tab" and menus, as well as git clone and cd jamulus seems a bit unnecessary/risky to me. Can we not bring it into line with the brevity of the other platforms?

When I originally did this doc, I removed a bunch of stuff I assumed experienced engineers/power users would see as noise, and we would be lumbered with checking in future...

COMPILING.md Outdated
`git submodule update --init`
* Open Jamulus.pro in Qt Creator
* In the Projects tab -> Build & Run It should appear the Android Qt, might be disabled. Enabled it if it is not enabled.
* Then you can connect your device and build or run the project.
Copy link
Member

Choose a reason for hiding this comment

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

Given the preceding detailed instructions on how to set up QT, is how to "connect your device and build or run the project" obvious? But see my earlier general comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @gilgongo for the feedback. Updated accordingly (and some bug fixes).

Copy link
Member

Choose a reason for hiding this comment

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

I'm worried that these instructions are going to mean we'll need to maintain them too much, and they seem to be (sort of) aimed at somebody who's never used QT before. This isn't the document for that.

Instead, is there anything here an experienced person who has compiled and run an Android QT app would not know? git submodule update --init after cloning the repo perhaps? Also, would we not need to list the necessary SDKs and NDKs?

Again, see the style of the instructions for other platforms in this document for the sort of thing I mean. The guide for iOS does not attempt to teach people how to use Qt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's really based on my personal experience with both, iOS and Android. I didn't have experience with either Android or iOS.
For iOS, it was kind of easy, I didn't have to deal with Qt Creator, just needed the Qt binaries to generate XCode project, and then I was good to go.
For Android though, Qt Creator and all the configurations seemed a lot more complicated.

Of course for an experienced person, I suppose they wouldn't need all these detailed instructions. I'll remove some more now.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know anything about Android really, but in general for this doc (and for various reasons), we need to just have the minimum we think is needed by somebody who has done this kind of thing before (as we do for the other platforms). We don't yet support Android, so I hope anyone expecting to use it after compiling will understand that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gilgongo should be good now :)

Copy link
Member

Choose a reason for hiding this comment

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

Excellent - thanks!

@gilgongo gilgongo merged commit a8bcd38 into jamulussoftware:master Jun 2, 2021
@pljones pljones added this to the Release 3.8.0 milestone Feb 19, 2022
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.

4 participants