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

Added infos on single values vs arrays and strings vs objects #304

Merged
merged 4 commits into from
Aug 13, 2018

Conversation

iherman
Copy link
Member

@iherman iherman commented Aug 13, 2018

The examples have also been extended for the various cases.

This answers to
#287 (comment)

Fixes #287


Preview | Diff

@mattgarrish
Copy link
Member

Content-wise I don't have any issue, but am wondering about organization. The new sections come before the section that references the properties, and link values are described in section 4. We're getting a bit all over the place.

I wonder if we should group them together, maybe like this:

2.3.3. Properties
2.3.3.1 Arrays and Single Values
2.3.3.2 Text Values and Objects
2.3.3.3 Links
2.3.3.3.1 PublicationLink Definition

?

@iherman
Copy link
Member Author

iherman commented Aug 13, 2018

Yes. I was not sure either. The problem is, however, that the section on publication types (ie, 3.2.4) actually makes use of the fact that the same property value can be an array or a single value, hence 3.2.2. preceding it...)

Do you think there is a better place for the current 3.2.2 and 3.2.3?

(B.t.w., the section numbers in your comments are odd; we are talking about section 3, right?)

@mattgarrish
Copy link
Member

the section numbers in your comments are odd; we are talking about section 3, right?

Yes, my ability to think is being disrupted by the sound of my neighbour sawing down trees.

the section on publication types (ie, 3.2.4) actually makes use of the fact that the same property value can be an array or a single value

The sections currently only refer to properties in section 4. Maybe that needs generalizing, too, then?

Maybe we can make a "Values" section that precedes the type declaration, or:

3.2.2. Values
3.2.2.1 Arrays and Single Values
3.2.2.2 Text Values and Objects
3.2.2.3 Links
3.2.2.3.1 PublicationLink Definition

@iherman
Copy link
Member Author

iherman commented Aug 13, 2018

Yes, my ability to think is being disrupted by the sound of my neighbour sawing down trees.

:-)

The sections currently only refer to properties in section 4. Maybe that needs generalizing, too, then?

Oops... You are right.

Maybe we can make a "Values" section that precedes the type declaration, or:
You mean moving sections 4.3 into 3, and regrouping with the current 3.2.2, 3.2.3. as one top level and put it after the current 3.2.1? Or maybe even after 3.1 as a almost top level?

I have a call in three minutes, so I will have to let it rest to after that (unless you beat me into it). At first glance it does make sense indeed.

@mattgarrish
Copy link
Member

I'll commit some changes shortly and let you have a look.

@mattgarrish
Copy link
Member

I merged the examples into a single unit with the last commit just to connect the thread a little more obviously. See if it all still makes sense.

@iherman
Copy link
Member Author

iherman commented Aug 13, 2018

Matt, I like what you did. I have made a commit with 2 minor editorial glitches (all mine); I would propose to merge this; after all, this is a purely editorial change.

@mattgarrish mattgarrish merged commit 4f6b03c into master Aug 13, 2018
@mattgarrish mattgarrish deleted the array-handling-update branch August 13, 2018 14:46
@HadrienGardeur
Copy link

I'm still uncomfortable with allowing a single string in readingOrder, resources or links, which is why I'm not a big fan of these examples.

I'm fine with providing this kind of flexibility to metadata, but I don't think we should allow them for more structural properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants