-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add the S3 icon #167
Add the S3 icon #167
Conversation
@LelouchFR Hi Baptiste, the readme file don't need to be updated anymore ? |
Hi @Daynlight can you provide me a guideline in order to resolve this issue ? |
It looks good for me icon added, documention updated. You can see your icon in list on your fork branch |
@Daynlight Okay nice ! Don't hesitate to reach me if there a problem with this icons or you want some improvements |
I think you need to cahnge S to s and auto generate readme you need use .github/readme-format.sh |
@Daynlight i have a problem i'm on mac and the command in order to generate the readme don't work /.github/readme-format.sh |
Can you send the pwd ? |
yes no problem /Users/francoislenne/skill-icons |
@LelouchFR insted of using you can use ls | grep -vE '(-dark|-light).svg$' it's more agnostic to the os or use grep if the os is mac in order to retrieve the OS do os=$(uname) and if it's a mac this value is equal to darwin the sub-system of macos |
I didn't use mac sorry :< |
README.md |
@Daynlight it wiil be ok right now |
oh no i speak too fast @Daynlight |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you just also make the #S3
to #s3
thanks, then it would be all set I guess. (just don't forget to merge main branch to be up to date and regenerate the readme for that (I'm sorry that it has be done on your side right now, but this should be the next problem fixed asap, becasue the script should be ran on pull_request to generate it, so I'm not gonna be doing one for every os or something like that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Add New Icon(s)
Icon Details
Notes
Checklist