Skip to content
This repository was archived by the owner on Mar 5, 2026. It is now read-only.

add new icon OBSERVABLE#281

Open
mattdzugan wants to merge 1 commit intoForkAwesome:masterfrom
mattdzugan:master
Open

add new icon OBSERVABLE#281
mattdzugan wants to merge 1 commit intoForkAwesome:masterfrom
mattdzugan:master

Conversation

@mattdzugan
Copy link
Copy Markdown

Proposing a new icon for Observable

image

had to create a second PR cuz i broke travis the first time 😬

@shinenelson shinenelson added the icon-request New icon request label Sep 4, 2021
Copy link
Copy Markdown
Member

@shinenelson shinenelson left a comment

Choose a reason for hiding this comment

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

@mattdzugan, thank you for adding the icon for Observable to Fork Awesome. It looks good 👌

However, in order for these changes to be merged in, can you please remove the generated font files from fonts and also src/icons/.fontcustom-manifest.json?

The duplication of package.json in src/icons is not required as well. Also, please resolve the merge conflict in src/icons/icons.yml by accepting both changes.

@mattdzugan
Copy link
Copy Markdown
Author

mattdzugan commented Sep 4, 2021

Will do thank you @shinenelson !!
I'll also try and update the contributing.md to make it more clear to folks like me exactly which files to include in the PRs

@shinenelson
Copy link
Copy Markdown
Member

I'll also try and update the contributing.md to make it more clear to folks like me exactly which files to include in the PRs

That would be great. It would be appreciated if you could create a separate pull request for it. We try and keep pull requests concise with what it does.

The reasoning behind that is because we follow a squash merge strategy and it would make sense to have only one item per pull request.

@ForkAwesome ForkAwesome locked and limited conversation to collaborators Sep 4, 2021
@ForkAwesome ForkAwesome unlocked this conversation Sep 4, 2021
@shinenelson
Copy link
Copy Markdown
Member

I am sorry about the locking. It was an accidental tap on mobile 😅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

icon-request New icon request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants