Skip to content

Navigation Menu

Sign in
Sign up

Settings icon for LDAP and encryption - #3244

Merged
MorrisJobke merged 3 commits into
master from
settings-icon
Jan 26, 2017
Merged

Settings icon for LDAP and encryption #3244
MorrisJobke merged 3 commits into
master from
settings-icon

Conversation

@MorrisJobke

@MorrisJobke MorrisJobke commented Jan 24, 2017

Copy link
Copy Markdown
Member 

* follow up to #3151
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>

Copy link
Copy Markdown

@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.

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

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

@nickvergessen nickvergessen left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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

@jancborchardt jancborchardt left a comment

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
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 dismissed their stale review January 25, 2017 10:36

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

MorrisJobke merged commit 6fe3cfa into master Jan 26, 2017
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

Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes
@ChristophWurst ChristophWurst ChristophWurst approved these changes
@blizzz blizzz blizzz approved these changes
@skjnldsv skjnldsv skjnldsv approved these changes
@jancborchardt jancborchardt jancborchardt left review comments

Assignees

No one assigned

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.

AltStyle によって変換されたページ (->オリジナル) /