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

Add SiYuan icon #10187

Merged
merged 7 commits into from
Jan 5, 2024
Merged

Conversation

Zuoqiu-Yingyi
Copy link
Contributor

image

GitHub stars: ~13.7k (2023-12-26)
Similarweb rank: ~516k (2023-12-26)

Checklist

  • I updated the JSON data in _data/simple-icons.json
  • I optimized the icon with SVGO or SVGOMG
  • The SVG viewbox is 0 0 24 24

Description

@github-actions github-actions bot added the new icon Issues or pull requests for adding a new icon label Dec 26, 2023
@Zuoqiu-Yingyi
Copy link
Contributor Author

It seems that the JSON Schema verify of aliases.loc property is not compatible with BCP 47 tag contains Script subtag such as zh-Hans and zh-Hant 👀?

"locale": {
"$id": "#locale",
"description": "A localized brand name",
"type": "object",
"patternProperties": {
"^[a-z]{2}-[A-Z]{2}$": {
"type": "string",
"description": "The local name of the brand"
}
},
"minProperties": 1,
"additionalProperties": false
},

REF: RFC 5646: Script Subtag
REF: https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry

@adamrusted
Copy link
Member

adamrusted commented Dec 31, 2023

It seems that the JSON Schema verify of aliases.loc property is not compatible with BCP 47 tag contains Script subtag such as zh-Hans and zh-Hant 👀?

Correct, the aliases.loc uses ISO 639 (the original version) for our language tags. As of right now, we don't differentiate between zh-Hans and zh-Hant - instead linking by a region, though we do only have this and this as examples within our codebase. If you could adapt this PR to suit ISO 639 for now, and perhaps open an issue up to discuss changing our language codes used - we can look to review and get this merged in.

@Zuoqiu-Yingyi
Copy link
Contributor Author

@adamrusted
To Be Reviewed.
This PR is currently ISO 639 adapted.

Copy link
Member

@adamrusted adamrusted left a comment

Choose a reason for hiding this comment

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

I'm personally happy with the icon itself, given how rubbish the source SVG is!
Given you've done a fair bit of work on this though, I'll ask another maintainer to review before we merge!

_data/simple-icons.json Show resolved Hide resolved
icons/siyuan.svg Outdated Show resolved Hide resolved
@PeterShaggyNoble
Copy link
Member

Merging this in based on Adam's approval above.

@PeterShaggyNoble PeterShaggyNoble merged commit 84596e3 into simple-icons:develop Jan 5, 2024
3 checks passed
@Zuoqiu-Yingyi Zuoqiu-Yingyi deleted the add-siyuan branch January 5, 2024 21:15
mondeja added a commit that referenced this pull request Jan 7, 2024
# New Icons

- Cooler Master (#10209)
- DeepCool (#10204)
- Make (#10165)
- SideQuest (#10202)
- SiYuan (#10187)
- Stagetimer (#10104)
- Victron Energy (#10198)
- Wyze (#10188)

# Updated Icons

- Affinity Designer (#10176)
- Affinity Photo (#10176)
- Affinity Publisher (#10176)
- Biome (#10203)
- Google Chrome (#10116)
- INFINITI (#10118)
- Opel (#10159)
- RTL (#10166)
- smart (#10168)
- STARZ (#10169)
- TELE 5 (#10170)
- Wish (#10171)

# Removed Icons

- Babylon.js (#9980)
- Hulu (#10048)
- Pepsi (#10086)
- Uno (#9973)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new icon Issues or pull requests for adding a new icon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants