Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use product name on rich document, if available #3971

Merged
merged 2 commits into from
May 23, 2019
Merged

Use product name on rich document, if available #3971

merged 2 commits into from
May 23, 2019

Conversation

tobiasKaminsky
Copy link
Member

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

@tobiasKaminsky tobiasKaminsky mentioned this pull request May 7, 2019
Merged
AndyScherzinger
AndyScherzinger reviewed May 8, 2019
View reviewed changes
src/main/res/values/strings.xml Outdated
@@ -793,7 +793,7 @@
<string name="stream_not_possible_headline">Internal streaming not possible</string>
<string name="stream_not_possible_message">Please download media instead or use external app.</string>
<string name="folder_already_exists">Folder already exists</string>
<string name="actionbar_open_as_richdocument">Open with Collabora</string>
<string name="actionbar_open_as_richdocument">Open with</string>
Copy link
Member

Choose a reason for hiding this comment

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

Or work with a pattern to have a parameter to not manually concatenate strings since we don't know how that works for all languages? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 changed

AndyScherzinger
AndyScherzinger requested changes May 8, 2019
View reviewed changes
Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

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

Please change the string to use a variable, probably better for translations :)

Else it looks fine to me.

@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #3971 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master   #3971      +/-   ##
===========================================
- Coverage      6.58%   6.53%   -0.06%     
  Complexity        1       1              
===========================================
  Files           322     322              
  Lines         30908   30927      +19     
  Branches       4408    4411       +3     
===========================================
- Hits           2036    2021      -15     
- Misses        28576   28612      +36     
+ Partials        296     294       -2
Impacted Files Coverage Δ Complexity Δ
...ain/java/com/owncloud/android/db/ProviderMeta.java 84.61% <ø> (ø) 0 <0> (ø) ⬇️
...wncloud/android/providers/FileContentProvider.java 19.13% <0%> (-0.23%) 0 <0> (ø)
...ava/com/owncloud/android/files/FileMenuFilter.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...loud/android/datamodel/FileDataStorageManager.java 11.93% <0%> (-0.03%) 0 <0> (ø)
...services/firebase/NCFirebaseInstanceIDService.java 0% <0%> (-87.5%) 0% <0%> (ø)
.../services/firebase/NCFirebaseMessagingService.java 0% <0%> (-7.15%) 0% <0%> (ø)
...xtcloud/client/preferences/AppPreferencesImpl.java 29.86% <0%> (-4.17%) 0% <0%> (ø)
...cloud/android/datamodel/ArbitraryDataProvider.java 11.53% <0%> (-1.29%) 0% <0%> (ø)
src/main/java/com/owncloud/android/MainApp.java 45.7% <0%> (-0.35%) 0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java 83.33% <0%> (+1.19%) 0% <0%> (ø) ⬇️

@codecov
Copy link

codecov bot commented May 9, 2019 •
edited
Loading

Codecov Report

Merging #3971 into master will decrease coverage by <.01%.
The diff coverage is 15%.

@@             Coverage Diff              @@
##             master    #3971      +/-   ##
============================================
- Coverage      14.3%   14.29%   -0.01%     
  Complexity        1        1              
============================================
  Files           329      329              
  Lines         30998    31017      +19     
  Branches       4424     4427       +3     
============================================
  Hits           4435     4435              
- Misses        25779    25795      +16     
- Partials        784      787       +3
Impacted Files Coverage Δ Complexity Δ
...ain/java/com/owncloud/android/db/ProviderMeta.java 84.61% <ø> (ø) 0 <0> (ø) ⬇️
...wncloud/android/providers/FileContentProvider.java 22.49% <0%> (-0.26%) 0 <0> (ø)
...ava/com/owncloud/android/files/FileMenuFilter.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...loud/android/datamodel/FileDataStorageManager.java 35.57% <100%> (+0.15%) 0 <0> (ø) ⬇️
...in/java/com/owncloud/android/datamodel/OCFile.java 61.92% <0%> (-1.38%) 0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java 82.14% <0%> (ø) 0% <0%> (ø) ⬇️

@nextcloud nextcloud deleted a comment May 9, 2019
@tobiasKaminsky tobiasKaminsky force-pushed the richDocumentsProductName branch from a72a0c5 to cd3f4fa Compare May 10, 2019 11:10
@tobiasKaminsky
Copy link
Member Author

I'll rebase this after #3989 is merged, as both touch database.

@AndyScherzinger
Copy link
Member

@tobiasKaminsky #3989 has been merged

@tobiasKaminsky tobiasKaminsky force-pushed the richDocumentsProductName branch from cd3f4fa to a28937d Compare May 16, 2019 07:47
@nextcloud nextcloud deleted a comment May 16, 2019
@AndyScherzinger
Copy link
Member

Lint score increased

@tobiasKaminsky tobiasKaminsky force-pushed the richDocumentsProductName branch from a28937d to 0e6a37d Compare May 17, 2019 05:23
@tobiasKaminsky
Copy link
Member Author

Lint score increased

This was due to the fact that I changed an existing translatable string and thus it is not yet updated on Transifex.
So I created just a new one, which results in a bit c&p for translators, but stays same for 3.6 and we do not get any strange conflicts, like Android trying to add a a parameter to a string without one.

@nextcloud nextcloud deleted a comment May 17, 2019
@AndyScherzinger AndyScherzinger force-pushed the richDocumentsProductName branch from 0e6a37d to 5b1b608 Compare May 17, 2019 15:41
@nextcloud nextcloud deleted a comment May 17, 2019
@tobiasKaminsky tobiasKaminsky force-pushed the richDocumentsProductName branch from 03435c1 to 54946a3 Compare May 22, 2019 09:01
@AndyScherzinger
Copy link
Member

@tobiasKaminsky can Rebase this one again?

@tobiasKaminsky
Use product name on rich document, if available
403a3e0 
use placeholder for string
reset to master-snapshot
fix analysis warning
use new translatable string
use var vor qa flavor too

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@tobiasKaminsky tobiasKaminsky force-pushed the richDocumentsProductName branch from 54946a3 to 403a3e0 Compare May 23, 2019 06:59
@AndyScherzinger
use proper qa build lib dependency
4197d3b 
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@nextcloud nextcloud deleted a comment May 23, 2019
@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/9393.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

297

Lint

TypemasterPR
Warnings5858
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings74
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings122
Security Warnings47
Dodgy code Warnings141
Total434

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings74
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings122
Security Warnings47
Dodgy code Warnings141
Total434

@tobiasKaminsky tobiasKaminsky merged commit fb6b11f into master May 23, 2019
@tobiasKaminsky tobiasKaminsky deleted the richDocumentsProductName branch May 23, 2019 08:37
@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.7.0 milestone May 23, 2019
tobiasKaminsky added a commit that referenced this pull request May 24, 2019
@tobiasKaminsky
daily dev 20190524
245ea82 
3b31841 Merge pull request #3969 from nextcloud/receivePushDelete
6074c93 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
f28007e OCFileListFragment: Only scroll to top if directory changed (#4058)
b061e4d Merge commit '51ee4d88fd0555a945a24b1ab3beba2ffdc79c06'
51ee4d8 handle silent delete/delete-all push notifications do not ignore talk messages (will only be sent if no talk app is installed) exclude generated Parcelable
fb6b11f Use product name on rich document, if available (#3971)
4197d3b use proper qa build lib dependency
8c06097 Only compare directories if necessary
403a3e0 Use product name on rich document, if available use placeholder for string reset to master-snapshot fix analysis warning use new translatable string use var vor qa flavor too
9b0a789 Merge pull request #4040 from nextcloud/ezaquarii/remove-findbugs
d6b4dee Remove legacy FindBugs tasks from build script
53c9c05 daily dev 20190523
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

Assignees
No one assigned
Labels
3. to review
Projects
None yet
Milestone
Nextcloud App 3.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@tobiasKaminsky @AndyScherzinger @nextcloud-android-bot