Skip to content

Conversation

@Jawnnypoo
Copy link
Member

Trying this again. This time, leaving the build only restriction

@Jawnnypoo Jawnnypoo requested review from natario1 and rogerhu January 19, 2018 15:59
@codecov
Copy link

codecov bot commented Jan 19, 2018

Codecov Report

Merging #774 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #774   +/-   ##
=========================================
  Coverage     53.42%   53.42%           
  Complexity     1748     1748           
=========================================
  Files           132      132           
  Lines         10295    10295           
  Branches       1428     1428           
=========================================
  Hits           5500     5500           
  Misses         4340     4340           
  Partials        455      455

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c27674...76b6cf3. Read the comment docs.

- pip install --user codecov
- yes | sdkmanager "platforms;android-27"
- mkdir "$ANDROID_HOME/licenses" || true
- echo "d56f5187479451eabf01fb78af6dfcb131a6481e" > "$ANDROID_HOME/licenses/android-sdk-license"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is that licence key?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a hash that signifies that you have read and agreed to the Android license. Here it is used in ButterKnife as well https://github.com/JakeWharton/butterknife/blob/master/.travis.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think it's necessary? i already added yes to the sdk manager.. besides Travis needs line 22 to pass

Copy link
Member Author

Choose a reason for hiding this comment

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

The downside to what you added is that you will have to continue to update it each time the SDK gets a bump, such as when API 28 releases. With this setup, it accepts all licenses for the Android SDK and therefore will install anything from the sdkmanager that it needs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just do this?

https://stackoverflow.com/a/45782695

yes | sdkmanager --licenses

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm it's using an older sdkmanager tools.

https://docs.travis-ci.com/user/languages/android/#Installing-a-newer-SDK-Platform-Tools-revision

When I run it locally, I see:

./sdkmanager
Usage: 
  sdkmanager [--uninstall] [<common args>] [--package_file <file>] [<packages>...]
  sdkmanager --update [<common args>]
  sdkmanager --list [<common args>]
  sdkmanager --licenses [<common args>]

Copy link
Contributor

@rogerhu rogerhu Jan 31, 2018

Choose a reason for hiding this comment

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

I think you need to specify a build tools, which should hopefully give you a newer sdkmanager.

No build-tools version is specified in android.components. Consider adding one of:
build-tools-27.0.3

The following versions are pre-installed:
build-tools-25.0.2

Copy link
Member Author

Choose a reason for hiding this comment

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

Well yeah thats the nice thing about doing it the way I have it now. It does not require changing the build tools version in the travis config each time you change it in code. On top of that, if we let the SDK decide which build tools to use based on the Android plugin, then we would not know the build tools version to install ahead of time. Instead, if we accept the license the way it is now, it would accept all build tools versions

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok well we should fix and change to use the more obvious sdkmanager line when Travis bumps the default build tools version.

@Jawnnypoo
Copy link
Member Author

Does this sound good @rogerhu ?

@Jawnnypoo Jawnnypoo merged commit c1fc653 into parse-community:master Jan 31, 2018
@Jawnnypoo Jawnnypoo deleted the travis branch January 31, 2018 23:05
bitterbit pushed a commit to bitterbit/Parse-SDK-Android that referenced this pull request Mar 9, 2018
* Build for any SDK on Travis.

* Use sdkmanager to approve licenses

* Revert "Use sdkmanager to approve licenses"

This reverts commit b51d415.
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