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

Utilizing openshift-logos-icon dependency #1743

Merged
merged 1 commit into from
Jun 26, 2017
Merged

Utilizing openshift-logos-icon dependency #1743

merged 1 commit into from
Jun 26, 2017

Conversation

rhamilto
Copy link
Member

Instead of local copy.

sg00dwin
sg00dwin approved these changes Jun 20, 2017
View reviewed changes
Copy link
Member

@sg00dwin sg00dwin left a comment

Choose a reason for hiding this comment

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

LGTM

@jwforres
Copy link
Member

were we not using the dist/styles/fonts/openshift-logos-icon.eot font file at all? looks like it was completely removed from the final dist output

did you run the grunt server in the dist mode to see if the logos still loaded correctly on all the browsers?

@rhamilto
Copy link
Member Author

were we not using the dist/styles/fonts/openshift-logos-icon.eot font file at all? looks like it was completely removed from the final dist output

No as the .eot is for IE6-8 and IE9 in compatibility mode.

did you run the grunt server in the dist mode to see if the logos still loaded correctly on all the browsers?

I did. But let me double check as you asking the question has cast doubt. ;-)

@jwforres
Copy link
Member

Ok :) the changes LGTM but I want to hold this till monday given the risk

@rhamilto
Copy link
Member Author

I did. But let me double check as you asking the question has cast doubt. ;-)

Double check is good.

screen shot 2017-06-20 at 1 34 04 pm

screen shot 2017-06-20 at 1 34 55 pm

@jwforres
Copy link
Member

[merge]

@openshift-bot
Copy link

[Test]ing while waiting on the merge queue

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2017
@openshift-bot
Copy link

Evaluated for origin web console test up to 4822d8f

@rhamilto rhamilto removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2017
@openshift-bot
Copy link

Evaluated for origin web console merge up to 4822d8f

@openshift-bot
Copy link

openshift-bot commented Jun 26, 2017 •
edited
Loading

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1598/) (Base Commit: 7bc6cbc) (PR Branch Commit: 4822d8f)

@openshift-bot
Copy link

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1597/) (Base Commit: ba1ba6d) (PR Branch Commit: 4822d8f)

@openshift-bot openshift-bot merged commit b14785b into openshift:master Jun 26, 2017
@rhamilto rhamilto deleted the openshift-logos-icon branch June 26, 2017 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sg00dwin sg00dwin sg00dwin approved these changes

@jwforres jwforres Awaiting requested review from jwforres

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rhamilto @jwforres @openshift-bot @sg00dwin