Skip to content

Update Windows ico#2367

Merged
cpuguy83 merged 1 commit intodocker:masterfrom
StefanScherer:new-icon
Mar 3, 2020
Merged

Update Windows ico#2367
cpuguy83 merged 1 commit intodocker:masterfrom
StefanScherer:new-icon

Conversation

@StefanScherer
Copy link
Copy Markdown
Member

@StefanScherer StefanScherer commented Mar 2, 2020

fixes #956

Signed-off-by: Stefan Scherer stefan.scherer@docker.com

- What I did

I've replaces the old Docker ico for the Windows exe files with a newer one.

I've also removed the docker.png file as I couldn't find any reference where it may be used. Maybe that was only an example of the ico file for people unable to view Windows ico files.

- How I did it

Picked it from an internal JIRA task. :-)

- How to verify it

Compile Windows docker.exe and check it in Windows Explorer and Sysinternals Process Monitor.

- Description for the changelog

Updated the Windows icon.

- A picture of a cute animal (not mandatory but encouraged)

systray-icon-win-light-frame0

See also moby/moby#40609

Signed-off-by: Stefan Scherer <stefan.scherer@docker.com>
@thaJeztah
Copy link
Copy Markdown
Member

The .png looks to be still the old logo? 😅

@thaJeztah
Copy link
Copy Markdown
Member

oh, never mind; the png is being deleted here, correct?

@StefanScherer
Copy link
Copy Markdown
Member Author

Yes, it's deleted in that commit. GitHub somehow doesn't show that better for such removed binary files.

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

SGTM

I assume it automatically picks up the right file (so changes needed to use the .ico instead of the .png, correct?)

@StefanScherer
Copy link
Copy Markdown
Member Author

The relevant change is in the .ico file which is used here https://github.com/docker/cli/blob/master/scripts/winresources/common.rc
The .png seems unused.

@thaJeztah
Copy link
Copy Markdown
Member

Ah, interesting. Actually recall looking at that at some point, but wasn't sure if the .png would implicitly be used somewhere

Copy link
Copy Markdown
Collaborator

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit 2a2b679 into docker:master Mar 3, 2020
@StefanScherer StefanScherer deleted the new-icon branch March 3, 2020 08:01
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 10, 2020
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.

Review winresources autogenerated package

3 participants