Cleanup project #2750

abelgardep · 2019-12-04T14:02:16Z

Lint warnings and errors cleanup.

Continuation of #2736
CC @hannesa2
To improve code quality, I recommend a lint check on every pull request.
Currently it's obvious disabled, because on master it fails ! Please enable lint check on CI

With this pull request you see in root the lint report lint-app-report.html

Btw, a lot of them should be solved later on, and after that step, you should prevent them in the future by severity="error" in lint.xml

jesmrec · 2019-12-10T12:08:25Z

Smoke test needed.

owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt

owncloudApp/src/main/java/third_parties/in/srain/cube/GridViewWithHeaderAndFooter.java

davigonz · 2019-12-16T11:33:36Z

owncloudApp/lint.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<lint>
+    <issue id="WrongConstant" severity="ignore" />


Do we still need to ignore these issues, haven't you fixed them?

davigonz · 2019-12-16T11:50:38Z

owncloudApp/src/main/java/com/owncloud/android/ui/activity/DrawerActivity.java

        intent.putExtra(Intent.EXTRA_SUBJECT, feedback);

-        intent.setData(Uri.parse(feedbackMail));
+        intent.setDataAndType(Uri.parse(feedbackMail), "text/plain");


Please move "text/plain" as a constant to somewhere. It's also used in FileOperationsHelper.java and other parts of the project.

I think that it could be an issue itself. Moving just this string to a constant looks weird since there are files like MimetypeIconUtil where every mime is hardcoded, and we will mix hardcoded and constants. I think it is better to standarize and move everything together, don't you think?

Ok, could you create an issue with the details? Thanks

davigonz · 2019-12-16T12:47:53Z

@abelgardep just a couple of comments

davigonz · 2019-12-17T17:54:23Z

I've noticed that when long pressing a file, the Select all and select inverse options appear as text instead of the share icon.

davigonz · 2019-12-18T11:10:52Z

@jesmrec code approved, ready to test

jesmrec · 2019-12-18T11:58:34Z

QA:

This is not a not a product feature, so i executed some tests to assure everything works:

As developers' suggestion, the passcode lock layout was also checked, with succesfully result.

Additionally, i ran lint to check the code status after the clean up with the following result:

> Task :owncloudApp:lint
Ran lint on variant debug: 520 issues found
Ran lint on variant release: 515 issues found
Wrote HTML report to file:..../lint-app-report.html

> Task :owncloudData:lint
Ran lint on variant release: 0 issues found
Ran lint on variant debug: 0 issues found

> Task :owncloudDomain:lint
Ran lint on variant release: 0 issues found
Ran lint on variant debug: 0 issues found

> Task :owncloudTestUtil:lint
Ran lint on variant release: 1 issues found
Ran lint on variant debug: 1 issues found
Wrote HTML report to file:.../lint-results.html
Wrote XML report to file:..../lint-results.xml

> Task :owncloud-android-library:owncloudComLibrary:lint
Ran lint on variant release: 1 issues found
Ran lint on variant debug: 1 issues found
Wrote HTML report to file:.../lint-results.html
Wrote XML report to file:..../lint-results.xml

a huge number of issues detected in owncloudApp module, that should be fixed if we want to move the lint checks to CI, as suggested in the first message.

Is it a good idea to open a new issue in which this issues are fixed? several issues? @abelgardep @davigonz

abelgardep · 2019-12-18T12:03:38Z

If you open the lint-app-report.html, you will see that most of those warnings are related with deprecated logs. I created an issue some days ago (#2757). I consider that we should move it forward as soon as possible.

jesmrec · 2019-12-18T12:16:53Z

Then, from my side we can now move forward.

abelgardep requested a review from davigonz December 4, 2019 14:04

abelgardep changed the base branch from new_arch/modularization_tests_cleanup to new_arch/modularization December 5, 2019 07:11

davigonz force-pushed the new_arch/modularization branch 2 times, most recently from 7f2f4ef to a316cff Compare December 5, 2019 09:07

abelgardep force-pushed the cleanup_project branch from 523c735 to 1c400fb Compare December 5, 2019 11:41

jesmrec added Estimation - 3 (M) Sprint labels Dec 10, 2019

jesmrec added this to the 2.15 milestone Dec 10, 2019

davigonz force-pushed the new_arch/modularization branch 2 times, most recently from 07d1ce5 to 456d2c5 Compare December 10, 2019 17:46

abelgardep mentioned this pull request Dec 11, 2019

lint check #2736

Closed

abelgardep changed the base branch from new_arch/modularization to master December 11, 2019 11:42

abelgardep force-pushed the cleanup_project branch from 1c400fb to c8e080e Compare December 11, 2019 11:44

hannesa2 suggested changes Dec 12, 2019

View reviewed changes

owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Show resolved Hide resolved

abelgardep force-pushed the cleanup_project branch from 1a89110 to 35089bf Compare December 12, 2019 10:38

hannesa2 suggested changes Dec 13, 2019

View reviewed changes

owncloudApp/src/main/java/third_parties/in/srain/cube/GridViewWithHeaderAndFooter.java Outdated Show resolved Hide resolved

hannesa2 approved these changes Dec 13, 2019

View reviewed changes

davigonz force-pushed the cleanup_project branch from 46549fc to 74802e0 Compare December 16, 2019 10:56

davigonz suggested changes Dec 16, 2019

View reviewed changes

davigonz mentioned this pull request Dec 16, 2019

Welcome new Android components and architecture #2351

Open

19 tasks

abelgardep force-pushed the cleanup_project branch from 09e7213 to 872fbf3 Compare December 17, 2019 13:39

davigonz force-pushed the cleanup_project branch from 872fbf3 to abdc2bf Compare December 17, 2019 17:02

abelgardep force-pushed the cleanup_project branch from ade9e89 to 0cc020f Compare December 18, 2019 09:22

abelgardep added 4 commits December 18, 2019 11:00

Remove unused layouts

eeeefed

Remove unused drawables

6b8595d

Remove unused assets

f19b9ca

Clean strings xml

4cbe037

abelgardep and others added 15 commits December 18, 2019 11:00

Minor fixes

7af63f9

Remove extra translation

1785d06

Use start/end instead of left/right

87867f3

Solve some lint errors

01ceb54

Lint warnings in Domain layer

45545ea

Lint warnings in Data layer

bafe5bd

lint check

c113a3d

Logging call to Timber instead of Log

0c04a96

Remove useless parent layout

0727108

Fix kotlin weak warnings

6a64239

Fix kotlin warnings

9165d76

Remove pointless logs

5054cfd

Replace ... with ellipsis character

07a3de7

Apply CR change

016c19f

Ignore lint error in file_actions_menu

2eff909

davigonz force-pushed the cleanup_project branch from 2d2c376 to 3e8b4e7 Compare December 18, 2019 10:01

Fix WrongConstant issue

1609e67

davigonz force-pushed the cleanup_project branch from 3e8b4e7 to 1609e67 Compare December 18, 2019 10:05

davigonz approved these changes Dec 18, 2019

View reviewed changes

davigonz added the code approved label Dec 18, 2019

jesmrec added the Approved by qa label Dec 18, 2019

davigonz merged commit 5ba41d5 into master Dec 18, 2019

davigonz deleted the cleanup_project branch December 18, 2019 12:32

jesmrec removed the Sprint label Jan 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup project #2750

Cleanup project #2750

abelgardep commented Dec 4, 2019 •

edited

Loading

jesmrec commented Dec 10, 2019

davigonz Dec 16, 2019

davigonz Dec 16, 2019

abelgardep Dec 17, 2019

davigonz Dec 17, 2019 •

edited

Loading

davigonz commented Dec 16, 2019

davigonz commented Dec 17, 2019

davigonz commented Dec 18, 2019

jesmrec commented Dec 18, 2019 •

edited

Loading

abelgardep commented Dec 18, 2019

jesmrec commented Dec 18, 2019

Cleanup project #2750

Cleanup project #2750

Conversation

abelgardep commented Dec 4, 2019 • edited Loading

jesmrec commented Dec 10, 2019

davigonz Dec 16, 2019

Choose a reason for hiding this comment

davigonz Dec 16, 2019

Choose a reason for hiding this comment

abelgardep Dec 17, 2019

Choose a reason for hiding this comment

davigonz Dec 17, 2019 • edited Loading

Choose a reason for hiding this comment

davigonz commented Dec 16, 2019

davigonz commented Dec 17, 2019

davigonz commented Dec 18, 2019

jesmrec commented Dec 18, 2019 • edited Loading

abelgardep commented Dec 18, 2019

jesmrec commented Dec 18, 2019

abelgardep commented Dec 4, 2019 •

edited

Loading

davigonz Dec 17, 2019 •

edited

Loading

jesmrec commented Dec 18, 2019 •

edited

Loading