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

new icon: sqlalchemy (original, original-wordmark, plain) #426

Merged
merged 5 commits into from
Jan 7, 2021
Merged

new icon: sqlalchemy (original, original-wordmark, plain) #426

merged 5 commits into from
Jan 7, 2021

Conversation

EnisMulic
Copy link
Contributor

Taken from #170

@Thomas-Boi Thomas-Boi added the bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger label Jan 5, 2021
@Thomas-Boi
Copy link
Member

Hello,

Here are the results of the peek process:
new_icons
screenshot_0
screenshot_1
screenshot_2

Everything looks good to me. However, I just want to confirm a few things with the amacado regarding the style="enable-background:new 0 0 128 128;" and x="0px" y="0px". When everything works out, I will merge your PR.

amacado
amacado requested changes Jan 5, 2021
View reviewed changes
devicon.json Outdated Show resolved Hide resolved
@amacado amacado added the feature:icon Use this label for pull requests when a new icon is ready to be added to the collection label Jan 5, 2021
@amacado
Copy link
Member

amacado commented Jan 5, 2021

@Thomas-Boi can you check if the aliases code is correct? I'm always confused with this.. 😆

Thomas-Boi
Thomas-Boi requested changes Jan 6, 2021
View reviewed changes
Copy link
Member

@Thomas-Boi Thomas-Boi left a comment

Choose a reason for hiding this comment

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

So I started a review about the aliases attribute base on what we currently use it for. However, it gets me thinking about whether this attribute is really necessary. I'll open a discussion on this topic to get everyone's feedback. Until then, we can either wait for this to resolve (maybe a week depends on what kind of feedback we get) or we can merge it when you fix the devicon.json.

devicon.json Outdated Show resolved Hide resolved
devicon.json Outdated Show resolved Hide resolved
@EnisMulic EnisMulic requested a review from Thomas-Boi January 7, 2021 07:32
@Thomas-Boi
Copy link
Member

Hey @EnisMulic thanks for the changes. Sorry for not merging this earlier, it kinda escaped my mind 😅

@Thomas-Boi Thomas-Boi merged commit bc6846d into devicons:develop Jan 7, 2021
@EnisMulic EnisMulic deleted the ReallyLiri/feature/icon/sqlalchemy branch January 7, 2021 17:58
@EnisMulic
Copy link
Contributor Author

It's okay. I didn't update the PR right away anyway.
Do you mind closing #170 as this was the last icon from that batch.

@Thomas-Boi
Copy link
Member

Sounds good, I'll close that PR 👍

@amacado amacado mentioned this pull request Jan 19, 2021
Merged
GCHQDeveloper926 pushed a commit to GCHQDeveloper926/devicon that referenced this pull request Dec 20, 2024
@Thomas-Boi
Merge pull request devicons#426 from EnisMulic/ReallyLiri/feature/ico…
b75cf98 
…n/sqlalchemy

new icon: sqlalchemy (original, original-wordmark, plain)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amacado amacado amacado requested changes

@Thomas-Boi Thomas-Boi Awaiting requested review from Thomas-Boi

Assignees

@Thomas-Boi Thomas-Boi

Labels
bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger feature:icon Use this label for pull requests when a new icon is ready to be added to the collection
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@EnisMulic @Thomas-Boi @amacado