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 schema of meta informations (2), licenses #184

Merged
merged 9 commits into from
Nov 12, 2020

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented Nov 6, 2020

Sequel of #182

これが終わると #181 が着手できるようです。

概要

前回の定例会議で口頭で議論した内容を踏まえ、metaのスキーマをアップデートしました。

  • 改名: violentUsage -> allowExcessivelyViolentUsage
    • 制限する暴力表現を成人向けのものであることをより明示的にする表現としました。
  • 改名: sexualUsage -> allowExcessivelySexualUsage
    • 制限する性的表現を成人向けのものであることをより明示的にする表現としました。
  • 削除: gameUsage
  • 変更: CommercialUsageType
    • PersonalCommercial に相当するenumを削除し、他のenumをそれに沿う形で改名しました。
  • 改名: politicalOrReligiousUsage -> allowPoliticalOrReligiousUsage
  • 削除: otherPermissionUrl
    • otherLicenseUrl はそのまま残っています。
  • 改名: modify -> modification
    • 名詞系が適切と思い、改名しました。
  • また、各プロパティの説明文を修正しました。

レビュー観点

  • 英語は適切でしょうか?
    • CommercialUsageType について、再検討する必要がある旨を口頭で行っていましたね。
    • modification 、どう思いますか

@0b5vr 0b5vr added the meta label Nov 6, 2020
@0b5vr 0b5vr added this to the v1.0 milestone Nov 6, 2020
@0b5vr 0b5vr requested review from ousttrue and hiroj November 6, 2020 08:04
@0b5vr 0b5vr self-assigned this Nov 6, 2020
@ousttrue
Copy link
Contributor

ousttrue commented Nov 6, 2020

補足します。

XXXUsage はもともと disallowallow にしようとしていました。
実装は2値しかないのを string enum にするのは煩雑なので、 bool にしてデフォルトを false にしてます(制限の厳しい方をデフォルト値にするのが妥当という考えです)。
XXXUsage よりは IsAllowXXX とかの方が bool には合うかもしれないです。

Copy link
Contributor

@ousttrue ousttrue left a comment

Choose a reason for hiding this comment

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

modification 👍

@0b5vr
Copy link
Contributor Author

0b5vr commented Nov 6, 2020

あ!必須パラメータもしくはデフォルト値いかが致しましょう。全部きびしいデフォルト値でoptionalとすれば良い気がしています。

@0b5vr
Copy link
Contributor Author

0b5vr commented Nov 6, 2020

あ!enumのcaseいかがしましょう。1.0の他の場所では統一してcamelCaseのようですが、こちらもcamelCaseで統一しちゃったほうが良い気がしています。

@ousttrue
Copy link
Contributor

ousttrue commented Nov 6, 2020

  • 厳しい default値 がいいと思います。false や 0 にしても問題ないところは require にしなくてもいいことがおおそうな気がします
  • VRM は lowerCamel 路線ぽいです

humanoid のボーン名がなんとなくそうだったのが最初かもしれないです。
本家は、GLTFの定数的な大文字スネークケース や 小文字などありました。それほどこだわりは無さそうです。

FMS_Cat added 3 commits November 10, 2020 11:41
excessivelyViolentUsage -> allowExcessivelyViolentUsage
excessivelySexualUsage -> allowExcessivelySexualUsage
politicalOrReligiousUsage -> allowPoliticalOrReligiousUsage
@0b5vr
Copy link
Contributor Author

0b5vr commented Nov 10, 2020

  • c97289f enumをcamelCaseにしました
  • 5c72641 各ライセンスフィールドにデフォルト値を追加しました
  • 2ceab10 boolのフィールドに allow を付与しました
    • excessivelyViolentUsage -> allowExcessivelyViolentUsage
    • excessivelySexualUsage -> allowExcessivelySexualUsage
    • politicalOrReligiousUsage -> allowPoliticalOrReligiousUsage

Copy link
Contributor

@ousttrue ousttrue left a comment

Choose a reason for hiding this comment

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

明瞭になってきました 👍

@ousttrue ousttrue merged commit 4693e45 into vrm-c:master Nov 12, 2020
@0b5vr 0b5vr deleted the meta-licenses branch November 12, 2020 09:16
0b5vr pushed a commit to pixiv/three-vrm that referenced this pull request Nov 12, 2020
0b5vr pushed a commit to pixiv/three-vrm that referenced this pull request Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants