Skip to content

Update sd6.svg #483

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.

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

Closed
wants to merge 1 commit into from
Closed

Update sd6.svg #483

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 23, 2021

  1. Removed the border in sd6.svg that popped up in the version submitted in Change the color of the icons on the Scientific Domains tab #479.
  2. Fixed the cropped parts of the original icon.

Copy link
Member

@InessaPawson InessaPawson left a comment

Choose a reason for hiding this comment

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

LGTM.

@MarsBarLee
Copy link
Contributor

It looks great! I can see this time you've made a separate branch instead of pushing from the master branch, which is also great.

Next time I recommend you make your changes in your original PR, in this case #479. You can do that by making your changes in the same branch. When you do 'git push origin', the new commits will show up in the original PR page, which you can see if you scroll down the page see your PR commit history.

If you keep your commits in one PR, that makes it easier for us to review all your changes together.

For this case, I don't think you need to do that, we can separately accept each PR,#479 and #483.

@seberg
Copy link
Member

seberg commented Oct 26, 2021

Silly nitpick and invisible when scaled down, but the new version looks just slightly squeezed vertically to me? (Making the circles not round)

@ghost
Copy link
Author

ghost commented Oct 26, 2021

Will look into it @seberg

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants