Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Android Studio 3.2.1 #198

Merged
merged 2 commits into from
Nov 29, 2018
Merged

Conversation

hannesa2
Copy link
Contributor

@hannesa2 hannesa2 commented Oct 24, 2018

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 31f974c to bd5daef Compare October 24, 2018 06:29
build.gradle Outdated
@@ -1,10 +1,10 @@
buildscript {
repositories {
jcenter()
Copy link
Contributor Author

@hannesa2 hannesa2 Oct 24, 2018

Choose a reason for hiding this comment

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

I guess this should be merged to make PR owncloud/android#2324 on main repo work.
Additional #199 needs it

This was referenced Oct 24, 2018
@davigonz davigonz added this to the 0.9.21 milestone Oct 24, 2018
@@ -27,6 +27,10 @@ dependencies {
android {
compileSdkVersion 26

defaultConfig {
minSdkVersion 14
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have you moved the minSdkVersion and not the compileSdkVersion? Is there any difference between setting this in build.gradle or in AndroidManifest.xml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

most recent Android Studio complain about having this values in AndroidManifest.xml It's a relict form Eclipse, which is not more supported since ~ 2016.09
It should be located in 'build.gradle'

It's related to #180 which removes Eclipse (or did it as I created the PR long time ago)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, can you move also the targetSdkVersion here?

@davigonz
Copy link
Contributor

Hi @hannesa2 , thank you for helping us with this dependencies update, I put a comment in the review.

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 1a20c10 to 4b125d4 Compare October 25, 2018 04:43
@@ -27,6 +27,10 @@ dependencies {
android {
compileSdkVersion 26

defaultConfig {
minSdkVersion 14
Copy link
Contributor

Choose a reason for hiding this comment

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

I see, can you move also the targetSdkVersion here?

@hannesa2
Copy link
Contributor Author

It's hard to smell whats too much or too less.
The focus of this PR is Android Studio 3.2.1 and I did only necessary (remove red Gradle hints) steps doing so.

But yes, I will do. But any further step should be done in additional PRs

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 4b125d4 to 43345b5 Compare October 25, 2018 07:17
hannesa2 added a commit to hannesa2/android-library that referenced this pull request Oct 25, 2018
@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 43345b5 to 2e9ef26 Compare October 25, 2018 07:29
hannesa2 added a commit to hannesa2/android-library that referenced this pull request Oct 25, 2018
@davigonz
Copy link
Contributor

@hannesa2 I have cloned your code and is not compiling because minSdkVersion of test_client project is still in AndroidManifest.xml, can you fix that? Thanks

@@ -29,9 +29,6 @@
android:versionName="1.0">

<uses-permission android:name="android.permission.INTERNET"/>
<uses-sdk
android:minSdkVersion="14"
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 deleted !

@hannesa2
Copy link
Contributor Author

hannesa2 commented Oct 25, 2018

@davigonz
"of test_client project is still in AndroidManifest.xml" ... sorry it's removed.
https://github.com/owncloud/android-library/pull/198/files#r228174090

Please double check your test, I'm receiving a

BUILD SUCCESSFUL in 49s
163 actionable tasks: 160 executed, 3 up-to-date

during ./gradlew clean build in lib and I'm able to start as well

image

Travis is green too

@hannesa2
Copy link
Contributor Author

Ahhh ! There is a sample_client and a test_client.
Wtf, this should someone understand, there is no hint in README and in Travis it's comment out.
image

It's dead code !

  1. It builds and starts without modification
    image
  2. It's dead code: so an other PR should use/fix it or simple delete it

@hannesa2
Copy link
Contributor Author

hannesa2 commented Oct 26, 2018

Btw test_client uses ActivityInstrumentationTestCase2
image

I see following way:

  1. you push merge on all merge-able PR (immediate without finding other dead stuff)
  2. I convert to Gradle-only (without any Eclipse)
  3. If a guy has fun, convert this tests to Espresso tests and use it in Travis (PR needs much longer because of ARM)
  4. Then we have a good base and I like to see it with right tags to use as a normal maven dependency
    via jitpack.io https://jitpack.io/#owncloud/android-library in any Android project

... but step by step. Start with 1

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 2e9ef26 to eec584c Compare October 27, 2018 10:23
hannesa2 added a commit to hannesa2/android-library that referenced this pull request Oct 27, 2018
@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from eec584c to 54484d9 Compare October 30, 2018 06:26
hannesa2 added a commit to hannesa2/android-library that referenced this pull request Oct 30, 2018
@hannesa2
Copy link
Contributor Author

hannesa2 commented Nov 7, 2018

Version 2.9 is out, what's about some progress here, or is it postponed to 20XX ?

@davigonz
Copy link
Contributor

davigonz commented Nov 8, 2018

Version 2.9 is out, what's about some progress here, or is it postponed to 20XX ?

@hannesa2 this is scheduled for 2.10, as you can see in the issue connected to this PR (owncloud/android#2327)

@davigonz davigonz removed this from the 0.9.21 milestone Nov 8, 2018
@davigonz davigonz mentioned this pull request Nov 8, 2018
@davigonz
Copy link
Contributor

davigonz commented Nov 8, 2018

Let's follow this way @hannesa2:

  1. Get rid of old ant tests via Get rid of old ant tests #211
  2. Merge this
  3. Remove Eclipse structure

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 54484d9 to 6574b67 Compare November 8, 2018 22:26
hannesa2 added a commit to hannesa2/android-library that referenced this pull request Nov 8, 2018
@davigonz
Copy link
Contributor

davigonz commented Nov 9, 2018

@hannesa2 old tests deletion got merged, can you please rebase this to move it forward?

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 6574b67 to d2e6327 Compare November 9, 2018 14:27
hannesa2 added a commit to hannesa2/android-library that referenced this pull request Nov 9, 2018
@hannesa2
Copy link
Contributor Author

hannesa2 commented Nov 9, 2018

The system is wrong, when you can successfully merge a PR and follow up is blocked because something is missing. Btw, this security/snyk is unknown to me
image

@davigonz
Copy link
Contributor

@DeepDiver1975 we are able to merge our PRs but we keep seeing the warning about security/snyk with test_client when it doesn't exist anymore, is there any way to avoid those checks?

Note: we don't need this PR to be merged yet 😄

@hannesa2
Copy link
Contributor Author

"Note: we don't need this PR to be merged yet 😄" ... maybe from technical reason when you happy with current situation. -I'm not-

Seeing the speed here, I'm not surprised, when it will merged, when 3.3 is out

@hannesa2 hannesa2 force-pushed the AndroidStudio-3.2.1 branch from 194feae to 94f3c37 Compare November 27, 2018 10:39
@davigonz davigonz merged commit 9502498 into owncloud:master Nov 29, 2018
@hannesa2 hannesa2 deleted the AndroidStudio-3.2.1 branch November 30, 2018 06:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants