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

Set 'ExplicitIgnorableItemLength = 0' to glTF elements to export mode… #158

Merged
merged 1 commit into from
Jan 31, 2019
Merged

Set 'ExplicitIgnorableItemLength = 0' to glTF elements to export mode… #158

merged 1 commit into from
Jan 31, 2019

Conversation

yutopp
Copy link
Contributor

@yutopp yutopp commented Jan 31, 2019

Resolve #157.

@yutopp yutopp requested a review from ousttrue January 31, 2019 06:32
@yutopp yutopp mentioned this pull request Jan 31, 2019
Closed
ousttrue
ousttrue reviewed Jan 31, 2019
View reviewed changes
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.

ExplicitIgnorableItemLength ?

@0b5vr
Copy link
Contributor

0b5vr commented Jan 31, 2019

空配列を落とす処理はここの次に続いているのでしょうか？

@yutopp
Copy link
Contributor Author

yutopp commented Jan 31, 2019

JsonSchemaの処理する関数の中で弾かれます。minItemsは1であり長さは0なので不正なJsonSchemaと判別されエラーになっていましたが、明示的に無視可能な長さを設定したのでエラーではなくスキップをするような感じです。

@0b5vr
Copy link
Contributor

0b5vr commented Jan 31, 2019

なるほど…… 実装としては、ExplicitIgnorableItemLength を新規に用意するよりかは、Optionalかつ空配列である場合を許容する、という形にしたほうが気持ち良い気がします。

@ousttrue
Copy link
Contributor

ousttrue commented Jan 31, 2019 •
edited
Loading

無視してよいかは人が判断する必要があるので ExplicitIgnorableItemLength で明示します。インタフェースは、今後に。

@yutopp yutopp added this to the v0.50 milestone Jan 31, 2019
@yutopp yutopp merged commit ac6e1b0 into master Jan 31, 2019
@yutopp yutopp deleted the fix/minItems_restrictions_for_gltf_elements branch January 31, 2019 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ousttrue ousttrue ousttrue approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.50
Development

Successfully merging this pull request may close these issues.
3 participants
@yutopp @0b5vr @ousttrue