Skip to content

Conversation

@tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Feb 21, 2018

Fix #2199
Needs: nextcloud/android-library#131

This needs nextcloud/server#8455, otherwise it will just show a regular folder icon.
Tested back to NC10.

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

@AndyScherzinger
Copy link
Member

Code looks good to me, build will fail until the lib PR is merged.

@nextcloud nextcloud deleted a comment Mar 2, 2018
@AndyScherzinger
Copy link
Member

@xXSTrikeXx please don't pull the master to dev branches, we keep em as-is and do a rebase before the merges :)

@xXSTrikeXx
Copy link
Contributor

xXSTrikeXx commented Mar 4, 2018

How do you rebase before the merges? I clicked in web on "update" to restart lint built, because I thought libraries were already merged to check if building is successful. @AndyScherzinger
I apologize.

@AndyScherzinger
Copy link
Member

@xXSTrikeXx you can't... rebases need to be done via git itself

@tobiasKaminsky
Copy link
Member Author

Rebase is done to clean commit history

@nextcloud nextcloud deleted a comment Mar 12, 2018
@mario
Copy link
Contributor

mario commented Mar 13, 2018

Please merge lib stuff.

@AndyScherzinger
Copy link
Member

Please merge lib stuff.

has already been merged yesterday :) 1.0.40 of the lib release still awaits the other open PRs ;)

<path
d="M1.46 2c-.25 0-.46.21-.46.46v11.08c0 .258.202.46.46.46h13.08c.258 0 .46-.202.46-.46V4.462c0-.25-.21-.463-.46-.463H8L6 2H1.46zm6.517 3.793h3.57v3.385L10.355 8.05 8.57 9.743l-1.19-1.13 1.786-1.69-1.19-1.13zm-2.38.564H7.38l.597.565h-2.38v4.514h4.758V9.178l.596.564v1.694c0 .312-.265.564-.595.564h-4.76c-.33 0-.595-.252-.595-.564V6.922c0-.313.266-.565.596-.565z"
fill-rule="evenodd" fill="#0082c9"/>
</svg> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

I know, but ... new line! :D

Copy link
Member

Choose a reason for hiding this comment

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

Fixed

mario
mario previously requested changes Mar 15, 2018
Copy link
Contributor

@mario mario left a comment

Choose a reason for hiding this comment

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

One minor thing.

@nextcloud nextcloud deleted a comment Mar 15, 2018
@mario
Copy link
Contributor

mario commented Mar 15, 2018 via email

@AndyScherzinger
Copy link
Member

👍 rebased, waiting for CI to complete

@nextcloud nextcloud deleted a comment Mar 15, 2018
@AndyScherzinger
Copy link
Member

@tobiasKaminsky test are failing because of the android-lib

@tobiasKaminsky
Copy link
Member Author

Depending if we want to get nextcloud/android-library#135 in it, we'll have to wait for the library release.

@AndyScherzinger
Copy link
Member

I think it be nice to also have the virus scan handling in the release but depends on @mario's feedback.

@nextcloud nextcloud deleted a comment Mar 23, 2018
@tobiasKaminsky
Copy link
Member Author

Lint is complaining here:

../../../src/main/res/drawable/folder_external.xml:8: Attribute fillType is only used in API level 24 and higher (current min is 14)
  5         xmlns:android="http://schemas.android.com/apk/res/android">
  6     <path
  7         android:fillColor="#0082c9"
  8         android:fillType="evenOdd"                                                                  
  9         android:pathData="M1.46,2c-0.25,0 -0.46,0.21 -0.46,0.46v11.08c0,0.258 0.202,0.46 0.46,0.46h13.08c0.258,0 0.46,-0.202 0.46,-0.46L15,4.462c0,-0.25 -0.21,-0.463 -0.46,-0.463L8,3.999L6,2L1.46,2zM7.977,5.793h3.57v3.385L10.355,8.05 8.57,9.743l-1.19,-1.13 1.786,-1.69 -1.19,-1.13zM5.597,6.357L7.38,6.357l0.597,0.565h-2.38v4.514h4.758L10.355,9.178l0.596,0.564v1.694c0,0.312 -0.265,0.564 -0.595,0.564h-4.76c-0.33,0 -0.595,-0.252 -0.595,-0.564L5.001,6.922c0,-0.313 0.266,-0.565 0.596,-0.565z"/>
 10 </vector>

Explanation:

This check finds attributes set in XML files that were introduced in a version newer than the oldest version targeted by your application (with the minSdkVersion attribute).
This is not an error; the application will simply ignore the attribute. However, if the attribute is important to the appearance or functionality of your application, you should consider finding an alternative way to achieve the same result with only available attributes, and then you can optionally create a copy of the layout in a layout-vNN folder which will be used on API NN or higher where you can take advantage of the newer attribute.
Note: This check does not only apply to attributes. For example, some tags can be unused too, such as the new element in layouts introduced in API 21.

I would keep it in and just adjust lint-result.

@nextcloud nextcloud deleted a comment Mar 23, 2018
@AndyScherzinger
Copy link
Member

@tobiasKaminsky fine by me while I don't understand what we need the fillType for.

@AndyScherzinger
Copy link
Member

removed the fill type since it isn't needed

@nextcloud nextcloud deleted a comment Mar 23, 2018
@tobiasKaminsky
Copy link
Member Author

tobiasKaminsky commented Mar 23, 2018

👍 thanks @AndyScherzinger

Approved with PullApprove

@AndyScherzinger
Copy link
Member

Rebased with cleaned up commit history

@nextcloud nextcloud deleted a comment Mar 23, 2018
@AndyScherzinger AndyScherzinger merged commit 0d63edb into master Mar 23, 2018
@AndyScherzinger AndyScherzinger deleted the externalFolderIcon branch March 23, 2018 10:46
@AndyScherzinger
Copy link
Member

Merged with approvals, counter had to be reset to 0 approvals which is closer to the real 2 approvals due to a disapproval lock down if ever disapproved at a certain point in time via GH review.

At all: Please just comment via GH review feature, don't set any approval status via GH ever!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants