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.

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

Update Core adapter logos and title to keep them uniform #7328

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dushmanta05
Copy link

@dushmanta05 dushmanta05 commented Apr 2, 2024

Updated the core adapter logos and titles in README to make them uniform.

It was looking something like this:

Screenshot 1

So changed the width height and logo URL to be more consistent with devicons.

Screenshot 2

@sailsbot
Copy link

sailsbot commented Apr 2, 2024

Thanks for submitting this pull request, @dushmanta05! We'll look at it ASAP.

In the mean time, here are some ways you can help speed things along:

  • discuss this pull request with other contributors and get their feedback. (Reactions and comments can help us make better decisions, anticipate compatibility problems, and prevent bugs.)
  • ask another JavaScript developer to review the files changed in this pull request. (Peer reviews definitely don't guarantee perfection, but they help catch mistakes and enourage collaborative thinking. Code reviews are so useful that some open source projects require a minimum number of reviews before even considering a merge!)
  • if appropriate, ask your business to sponsor your pull request. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
  • make sure you've answered the "why?" (Before we can review and merge a pull request, we feel it is important to fully understand the use case: the human reason these changes are important for you, your team, or your organization.)

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

@dushmanta05
Copy link
Author

Hey @DominusKelvin!
I know this isn't a major issue, but I would love to get your review on this. Thank you for your time and effort.

@dushmanta05 dushmanta05 deleted the patch-1 branch July 30, 2024 12:36
@dushmanta05 dushmanta05 restored the patch-1 branch August 18, 2024 18:31
@dushmanta05 dushmanta05 reopened this Aug 18, 2024
@sailsbot
Copy link

Oh hey again, @dushmanta05. Now that this pull request is reopened, it's on our radar. Please let us know if there's any new information we should be aware of!


Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

<a target="_blank" href="http://www.postgresql.org/"><img width="50" title="PostgreSQL" src="http://i.imgur.com/OSlDDKv.png"/></a>&nbsp; &nbsp; &nbsp; &nbsp;
<a target="_blank" href="http://www.mongodb.org/"><img width="100" title="MongoDB" src="http://i.imgur.com/bC2j13z.png"/></a>&nbsp; &nbsp; &nbsp; &nbsp;
<a target="_blank" href="http://redis.io/"><img width="75" title="Redis" src="http://i.imgur.com/dozv0ub.jpg"/></a>&nbsp; &nbsp; &nbsp; &nbsp;
<a target="_blank" href="http://www.mysql.com/"><img width="55" title="MySQL" src="https://cdn.jsdelivr.net/gh/devicons/devicon@latest/icons/mysql/mysql-original-wordmark.svg" alt="MySQL"/></a>&nbsp; &nbsp; &nbsp; &nbsp;
Copy link

Choose a reason for hiding this comment

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

Hello @dushmanta05
Good PR 👍
The changes are misleading as the PR title meant to update only logo URLs. But, the content is changed.
I noticed the MySQL is changed from
alt="Powered by MySQL" title="sails-mysql: MySQL adapter for Sails"
to
alt="MySQL" title="MySQL"

Either change PR title to match the changes or Refactor the changes to match PR title.

Copy link
Author

@dushmanta05 dushmanta05 Sep 13, 2024

Choose a reason for hiding this comment

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

Thank you. I changed the title to make it uniform with other logos and titles; I'll update it in the PR title. Thanks again.

@dushmanta05 dushmanta05 changed the title Update Core adapter logos Update Core adapter logos and title to keep them uniform Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants