remove Eclipse structure #180

hannesa2 · 2017-12-17T09:23:28Z

This PR is related to owncloud/android#2076 to make it work. Nothing more.

in submodule I didn't take care about test and sample project, they are in old Eclipse structure and converting them to 'normal' Android structure causes DEX errors,...
This can be done in a additional PR

CLAassistant · 2017-12-17T09:23:34Z

All committers have signed the CLA.

hannesa2 · 2017-12-17T09:30:08Z

Ahh, again "Contributor License Agreement"
I did it for main repo, but as I see you want it for submodule separated and additional too ! wtf
I'm fine when you do a git commit --amend --reset-author on my commit to get rid of it

hannesa2 · 2018-10-24T06:21:46Z

Does it makes sense to maintain this PR ?
I just see this repo has a lot of outdated stuff and is in bad maintenance mode, but nothing from me will accepted in main repo (and probably in this too)
I made some PR, nobody merged it and after month somebody close it

I just want save my time but on the other hand I want some features, but before the complete project needs some modernization, first step should be a removement of Eclipse to do other stuff faster

michaelstingl · 2018-10-24T09:05:23Z

@hannesa2 Thank you for your feedback! Please join us in the team chat to discuss the next steps: https://talk.owncloud.com/channel/mobile (choose GitHub for login)

hannesa2 · 2018-11-30T06:44:18Z

When you merge the other open PR's I can continue here.
Btw if the maintainer of this #186 doesn't solve the issues, it could be done by me after merge this PR

davigonz · 2018-12-12T16:48:30Z

Let's move this forward @hannesa2, can you please rebase and solve the conflicts?

By the way, having a look at the Files changed tab, it seems that this PR is not going to remove the eclipse structure as owncloud/android#2076 , only 7 files in here while 822 files in the app PR

hannesa2 · 2018-12-12T18:38:29Z

@davigonz Exactly, currently It's only homeopathic. I will do the big change....

hannesa2 · 2018-12-12T19:09:06Z

I fixed Lint errors, but ignore Lint warnings to be able to run command /gradlew clean build
But an ideal library has no warnings at all !

hannesa2 · 2018-12-12T19:33:49Z

btw this "Files changed" tab is not very clear

... and so on ...

davigonz · 2018-12-13T08:48:33Z

owncloudComLib/src/main/java/lib/resources/status/GetRemoteStatusOperation.java

@@ -52,7 +52,7 @@
 * @author David González Verdugo
 */

-public class GetRemoteStatusOperation extends RemoteOperation<OwnCloudVersion> {
+public class GetRemoteStatusOperation extends RemoteOperation<com.owncloud.android.lib.resources.status.OwnCloudVersion> {


Think com.owncloud.android.lib doesn't exist anymore, are you sure this is properly compiling?

Anyway, what are you using to do this migration? Any automatic tool or is manually?

Android Studio does it during move of class locations

it's still there, but you are right the package name here is obsolete, it's not necessary

Think com.owncloud.android.lib doesn't exist anymore, are you sure this is properly compiling?

https://travis-ci.org/owncloud/android-library/builds/467152043?utm_source=github_status&utm_medium=notification

davigonz · 2018-12-13T08:50:34Z

settings.gradle

@@ -1,2 +1,2 @@
-include ':'
+include ':owncloudComLib'


I would keep the name android-library and also the middle packages com.owncloud.android instead of directly lib

Thinking about there is an other unknow app using this lib next to other libs (not Owncloud itself), and any included lib is called android-library
I have it in a current project, the libs are called sdk and 'library', ... -sad-
With android you know an obsolete information
With library you know it's an library ...congratulations

Thinking about there is an other unknow app using this lib next to other libs (not Owncloud itself), and any included lib is called android-library
I have it in a current project, the libs are called sdk and 'library', ... -sad-

I agree with this point, you are right, android-library is not enough but is the name you have set in your ownCloud Library fork if I'm not wrong.

With android you know an obsolete information

Why android is an obsolete information here? This library is clearly an Android project

What about owncloud-android-library? It's what we currently have in the official project and is self-explanatory.

CC/ @jesmrec @michaelstingl

Thinking about there is an other unknow app using this lib next to other libs (not Owncloud itself), and any included lib is called android-library
I have it in a current project, the libs are called sdk and 'library', ... -sad-
With android you know an obsolete information
With library you know it's an library ...congratulations

with the current owncloud-android-library you know that this is an android based library to be used in ownCloud, you have all the information and do not need any other information. This is my HO. I do not see any reason to switch the modules' names.

ok, I will apply owncloud-android-library but keep in mind, based on the name you don't know does is deliver GUI elements, communication stuff, general helpers or what else

I still do not get what additional information the previous name you had - owncloudComLib - provides in comparison with owncloud-android-library. Com means communication? If so, you could have detailed it before, I understood it as a domain

@davigonz anyway, names are the most complex issue in software development. I followed the suggestion from @jesmrec

davigonz · 2018-12-14T09:10:59Z

@hannesa2 please include again the packages com.owncloud.android before lib because there's many errors like the next one everywhere.

hannesa2 · 2018-12-14T09:14:46Z

Indeed, thanks for finding. Unfortunately it builds anyway. I will fix

hannesa2 · 2018-12-14T09:18:18Z

@davigonz packages are fixed

davigonz · 2018-12-14T09:51:02Z

owncloud-android-library/build.gradle

+
+    defaultConfig {
+        minSdkVersion 14
+        targetSdkVersion 28


Upgrading the sdk version is not always a trivial task and we do it carefully, with a specific issue to test everything and ensure nothing is broken, so I would keep 26 in compileSdkVersion and targetSdkVersion here and in sample client. Android P migration will be faced separately in owncloud/android#2283

It violets my principe "never go back" and don't introduce Lint warnings intentionally

But I want to do the shit done, so it's 26 again

davigonz · 2018-12-14T09:51:24Z

@hannesa2 one tiny change more and we are done

on review request

davigonz · 2018-12-14T11:40:30Z

Merged, let's continue with owncloud/android#2076 then

hannesa2 · 2018-12-14T12:15:23Z

@davigonz cool and please don't forget #203

hannesa2 force-pushed the update_android_tool4removeEclipse branch from 32580c4 to f989b9d Compare December 17, 2017 09:31

hannesa2 closed this Dec 17, 2017

hannesa2 reopened this Oct 24, 2018

This was referenced Oct 24, 2018

Android Studio 3.2.1 #198

Merged

Remove eclipse owncloud/android#2076

Merged

hannesa2 force-pushed the update_android_tool4removeEclipse branch 2 times, most recently from 1a6ccb6 to 83b22c9 Compare December 12, 2018 18:45

hannesa2 added 2 commits December 12, 2018 20:16

remove Eclipse only files

137e489

change to Gradle file structure

56e0031

hannesa2 force-pushed the update_android_tool4removeEclipse branch from 83b22c9 to 56e0031 Compare December 12, 2018 19:17

hannesa2 mentioned this pull request Dec 12, 2018

add missing permission in library #202

Closed

davigonz suggested changes Dec 13, 2018

View reviewed changes

apply codereviews

9fb69ce

fix packages

0eb071b

davigonz suggested changes Dec 14, 2018

View reviewed changes

revert to android 26

982dbb2

on review request

davigonz approved these changes Dec 14, 2018

View reviewed changes

davigonz added the code approved label Dec 14, 2018

davigonz merged commit c9337dc into owncloud:master Dec 14, 2018

hannesa2 deleted the update_android_tool4removeEclipse branch December 14, 2018 12:04

hannesa2 mentioned this pull request Dec 14, 2018

Update Kotlin #199

Closed

davigonz mentioned this pull request Feb 4, 2019

Test every PR ? #230

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove Eclipse structure #180

remove Eclipse structure #180

hannesa2 commented Dec 17, 2017

CLAassistant commented Dec 17, 2017 •

edited

Loading

hannesa2 commented Dec 17, 2017 •

edited

Loading

hannesa2 commented Oct 24, 2018

michaelstingl commented Oct 24, 2018

hannesa2 commented Nov 30, 2018

davigonz commented Dec 12, 2018

hannesa2 commented Dec 12, 2018 •

edited

Loading

hannesa2 commented Dec 12, 2018

hannesa2 commented Dec 12, 2018

davigonz Dec 13, 2018

davigonz Dec 13, 2018 •

edited

Loading

hannesa2 Dec 13, 2018

hannesa2 Dec 13, 2018

hannesa2 Dec 13, 2018

davigonz Dec 13, 2018

hannesa2 Dec 13, 2018

davigonz Dec 13, 2018 •

edited

Loading

jesmrec Dec 13, 2018

hannesa2 Dec 13, 2018

davigonz Dec 14, 2018

hannesa2 Dec 14, 2018

davigonz commented Dec 14, 2018

hannesa2 commented Dec 14, 2018

hannesa2 commented Dec 14, 2018

davigonz Dec 14, 2018

hannesa2 Dec 14, 2018

davigonz commented Dec 14, 2018

davigonz commented Dec 14, 2018

hannesa2 commented Dec 14, 2018

remove Eclipse structure #180

remove Eclipse structure #180

Conversation

hannesa2 commented Dec 17, 2017

CLAassistant commented Dec 17, 2017 • edited Loading

hannesa2 commented Dec 17, 2017 • edited Loading

hannesa2 commented Oct 24, 2018

michaelstingl commented Oct 24, 2018

hannesa2 commented Nov 30, 2018

davigonz commented Dec 12, 2018

hannesa2 commented Dec 12, 2018 • edited Loading

hannesa2 commented Dec 12, 2018

hannesa2 commented Dec 12, 2018

Choose a reason for hiding this comment

davigonz Dec 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz Dec 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz commented Dec 14, 2018

hannesa2 commented Dec 14, 2018

hannesa2 commented Dec 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz commented Dec 14, 2018

davigonz commented Dec 14, 2018

hannesa2 commented Dec 14, 2018

CLAassistant commented Dec 17, 2017 •

edited

Loading

hannesa2 commented Dec 17, 2017 •

edited

Loading

hannesa2 commented Dec 12, 2018 •

edited

Loading

davigonz Dec 13, 2018 •

edited

Loading

davigonz Dec 13, 2018 •

edited

Loading