Skip to content

Conversation

@MorrisJobke
Copy link
Member

* follow up to #3151

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke added the 3. to review Waiting for reviews label Jan 24, 2017
@MorrisJobke MorrisJobke added this to the Nextcloud 12.0 milestone Jan 24, 2017
@mention-bot
Copy link

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @blizzz and @nickvergessen to be potential reviewers.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

fancy!

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 25, 2017
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

There was 1 failure:
1) OCA\User_LDAP\Tests\Settings\SectionTest::testGetIcon
Expectation failed for method name is equal to <string:imagePath> when invoked 1 time(s)
Parameter 1 for invocation OCP\IURLGenerator::imagePath('user_ldap', 'app-dark.svg') does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'app.svg'
+'app-dark.svg'
/drone/src/github.com/nextcloud/server/apps/user_ldap/lib/Settings/Section.php:80
/drone/src/github.com/nextcloud/server/apps/user_ldap/tests/Settings/SectionTest.php:74

@nickvergessen nickvergessen added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Jan 25, 2017
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Jan 25, 2017
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Can you use the icon for multiple people instead of a single person? :) That makes more sense.

@nickvergessen
Copy link
Member

Well that is the current app icon:

apps_-nodesign-_2017-01-25_11 32 20

So if we change this, we should change both.

@jancborchardt jancborchardt dismissed their stale review January 25, 2017 10:36

Right, probably good after all since otherwise it would be confused with Contacts.

@MorrisJobke MorrisJobke merged commit 6fe3cfa into master Jan 26, 2017
@MorrisJobke MorrisJobke deleted the settings-icon branch January 26, 2017 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants