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

Improve collections docs #5691

Merged
merged 4 commits into from Jan 17, 2017

Conversation

Projects
None yet
5 participants
@tomjoht
Contributor

tomjoht commented Dec 26, 2016

See #5630 for more details on the update.

@jekyll/documentation

Improve collections docs
See #5630 for more details on the update. 

@jekyll/documentation
@DirtyF
@ashmaroli

not very sure why you changed e.g. to for example. Seems like just a personal preference.
But there are some other suggestions you may have to consider

Show outdated Hide outdated docs/_docs/collections.md
@@ -7,12 +7,18 @@ permalink: /docs/collections/
Not everything is a post or a page. Maybe you want to document the various
methods in your open source project, members of a team, or talks at a
conference. Collections allow you to define a new type of document that behave
like Pages or Posts do normally, but also have their own unique properties and
like Pages or Posts do normally but which also have their own unique properties and

This comment has been minimized.

@ashmaroli

ashmaroli Dec 26, 2016

Member

IMO, this change is wrong..

Collections allow you to define a new type of document that behave like Pages or Posts do normally but which also have their own unique properties..

just doesn't flow well..

@ashmaroli

ashmaroli Dec 26, 2016

Member

IMO, this change is wrong..

Collections allow you to define a new type of document that behave like Pages or Posts do normally but which also have their own unique properties..

just doesn't flow well..

This comment has been minimized.

@tomjoht

tomjoht Dec 27, 2016

Contributor

okay, changed it back.

@tomjoht

tomjoht Dec 27, 2016

Contributor

okay, changed it back.

Show outdated Hide outdated docs/_docs/collections.md
documents. YAML Front Matter is read in as data if it exists, and everything
after it is stuck in the Document's `content` attribute. If no YAML Front
after it is available in the document's `content` attribute. If no YAML Front

This comment has been minimized.

@ashmaroli

ashmaroli Dec 26, 2016

Member
- after it is available in the document's `content` attribute.
+ after it, is accessible via the document's `content` attribute. (e.g. `{{ post.content }}`)
@ashmaroli

ashmaroli Dec 26, 2016

Member
- after it is available in the document's `content` attribute.
+ after it, is accessible via the document's `content` attribute. (e.g. `{{ post.content }}`)

This comment has been minimized.

@tomjoht

tomjoht Dec 27, 2016

Contributor

updated

@tomjoht

tomjoht Dec 27, 2016

Contributor

updated

Show outdated Hide outdated docs/_docs/collections.md
## Liquid Attributes
### Collections
Each collection is accessible via the `site` Liquid variable. For example, if
Each collection is accessible through the `site` variable. For example, if

This comment has been minimized.

@ashmaroli

ashmaroli Dec 26, 2016

Member

👎 on changing via to through

@ashmaroli

ashmaroli Dec 26, 2016

Member

👎 on changing via to through

This comment has been minimized.

@tomjoht

tomjoht Dec 27, 2016

Contributor

"via" and "e.g." and other Latinates are discouraged in technical style guides (such as the Microsoft Manual of Style). They may be familiar in the U.S., but for non-native speakers in other countries reading English, Latinates introduce more complexity. of course it's somewhat trivial here. seems like both are equally readable.

@tomjoht

tomjoht Dec 27, 2016

Contributor

"via" and "e.g." and other Latinates are discouraged in technical style guides (such as the Microsoft Manual of Style). They may be familiar in the U.S., but for non-native speakers in other countries reading English, Latinates introduce more complexity. of course it's somewhat trivial here. seems like both are equally readable.

This comment has been minimized.

@parkr

parkr Dec 29, 2016

Member

I'm fine with either. Latinates will be very understandable to folks who speak Latin-derived languages, but may prove a barrier for many others. Perhaps removing this sentence altogether and replacing it with:

Each collection is accessible as a field on the site Liquid variable.

@parkr

parkr Dec 29, 2016

Member

I'm fine with either. Latinates will be very understandable to folks who speak Latin-derived languages, but may prove a barrier for many others. Perhaps removing this sentence altogether and replacing it with:

Each collection is accessible as a field on the site Liquid variable.

This comment has been minimized.

@tomjoht

tomjoht Dec 29, 2016

Contributor

i don't mean to cause fuss over a point that matters so little. the benefit of latin abbreviations is that they do save space, and developers like things as brief as possible usually.

@tomjoht

tomjoht Dec 29, 2016

Contributor

i don't mean to cause fuss over a point that matters so little. the benefit of latin abbreviations is that they do save space, and developers like things as brief as possible usually.

Made requested updates on this topic
Made minor grammar updates
@tomjoht

This comment has been minimized.

Show comment
Hide comment
@tomjoht

tomjoht Dec 27, 2016

Contributor

I made the requested updates. Submitting for re-review.

Contributor

tomjoht commented Dec 27, 2016

I made the requested updates. Submitting for re-review.

Show outdated Hide outdated docs/_docs/collections.md
@@ -51,7 +51,7 @@ defaults:
Create a corresponding folder (for example, `<source>/_my_collection`) and add
documents. YAML Front Matter is read in as data if it exists, and everything
after it is available in the document's `content` attribute. If no YAML Front
after it is accessible via the document's `content` attribute. If no YAML Front

This comment has been minimized.

@ashmaroli

ashmaroli Dec 27, 2016

Member

i had also suggested a comma following "after it": after it, is accessible

@ashmaroli

ashmaroli Dec 27, 2016

Member

i had also suggested a comma following "after it": after it, is accessible

This comment has been minimized.

@tomjoht

tomjoht Dec 29, 2016

Contributor

putting a comma between a noun ("everything") and a verb ("is") would make this grammatically incorrect. https://www.grammarly.com/handbook/punctuation/comma/4/comma-with-subjects-and-verbs/

@tomjoht

tomjoht Dec 29, 2016

Contributor

putting a comma between a noun ("everything") and a verb ("is") would make this grammatically incorrect. https://www.grammarly.com/handbook/punctuation/comma/4/comma-with-subjects-and-verbs/

This comment has been minimized.

@ashmaroli

ashmaroli Dec 29, 2016

Member

I don't want to continue arguing over this.. just one last attempt..
Read through the following:

"Everything after it is available online" — this denotes a point-in-time after something is available online. (the noun is just Everything)
"Everything after it, is available online" — this denotes a point-in-time right now and beyond. (the noun is Everything after it referring to whatever there is, after the object it. (it == FrontMatter dashes in our original context.)

Upvote if I'm in the right, else downvote. 😃

@ashmaroli

ashmaroli Dec 29, 2016

Member

I don't want to continue arguing over this.. just one last attempt..
Read through the following:

"Everything after it is available online" — this denotes a point-in-time after something is available online. (the noun is just Everything)
"Everything after it, is available online" — this denotes a point-in-time right now and beyond. (the noun is Everything after it referring to whatever there is, after the object it. (it == FrontMatter dashes in our original context.)

Upvote if I'm in the right, else downvote. 😃

This comment has been minimized.

@tomjoht

tomjoht Dec 29, 2016

Contributor

sorry, i just don't see the logic there. if you were omitting words, then the comma could work. Such as, "YAML Front Matter is read in as data if it exists, and everything
after, in the Document's content attribute." In that case, the "," stands for "is read in as."

There is also a slight pause in your phrasing, so I can see why you might want a comma there to indicate the pause.

I think the overall sentence is a little janky anyway. The problem is that "it" is ambiguous (does it refer to Front Matter or data -- at first read, it's not all that clear), and there are 2 "it" references.

Here's how I would fix that:

Create a corresponding folder (e.g. <source>/_my_collection) and add
documents. YAML front matter is processed if the front matter exists, and everything
after the front matter is pushed into the Document's content attribute. If no YAML Front
Matter is provided, Jekyll will not generate the file in your collection.

I think "read in as data" is kind of vague, and "is stuck into" is also an odd word choice. Do you like this new paragraph better, or should I keep it as is?

I don't mean to bikeshed this point. Probably doesn't matter a whole lot either way. I'll commit this update and you can downvote it if you hate it.

@tomjoht

tomjoht Dec 29, 2016

Contributor

sorry, i just don't see the logic there. if you were omitting words, then the comma could work. Such as, "YAML Front Matter is read in as data if it exists, and everything
after, in the Document's content attribute." In that case, the "," stands for "is read in as."

There is also a slight pause in your phrasing, so I can see why you might want a comma there to indicate the pause.

I think the overall sentence is a little janky anyway. The problem is that "it" is ambiguous (does it refer to Front Matter or data -- at first read, it's not all that clear), and there are 2 "it" references.

Here's how I would fix that:

Create a corresponding folder (e.g. <source>/_my_collection) and add
documents. YAML front matter is processed if the front matter exists, and everything
after the front matter is pushed into the Document's content attribute. If no YAML Front
Matter is provided, Jekyll will not generate the file in your collection.

I think "read in as data" is kind of vague, and "is stuck into" is also an odd word choice. Do you like this new paragraph better, or should I keep it as is?

I don't mean to bikeshed this point. Probably doesn't matter a whole lot either way. I'll commit this update and you can downvote it if you hate it.

This comment has been minimized.

@ashmaroli

ashmaroli Dec 29, 2016

Member

see how many times you repeated front matter..? it was used as a preposition pronoun for the same front matter.

@ashmaroli

ashmaroli Dec 29, 2016

Member

see how many times you repeated front matter..? it was used as a preposition pronoun for the same front matter.

This comment has been minimized.

@tomjoht

tomjoht Dec 29, 2016

Contributor

I only repeated it to avoid the ambiguity. Feel free to change this sentence to make it read how you think it sounds best. I'm actually heading out of town for a few days, so my communication will be spotty. (I didn't want to bring up capitalization issues with Front Matter versus front matter, or document versus Document, but I'll try to include something in a style guide at some future point so we maintain consistency.)

@tomjoht

tomjoht Dec 29, 2016

Contributor

I only repeated it to avoid the ambiguity. Feel free to change this sentence to make it read how you think it sounds best. I'm actually heading out of town for a few days, so my communication will be spotty. (I didn't want to bring up capitalization issues with Front Matter versus front matter, or document versus Document, but I'll try to include something in a style guide at some future point so we maintain consistency.)

@parkr

Thanks again! Just a few comments here.

Show outdated Hide outdated docs/_docs/collections.md
collections:
my_collection:
output: true
permalink: /awesome/:path/:title.output_ext

This comment has been minimized.

@parkr

parkr Dec 29, 2016

Member

:title.:output_ext here – there is a colon missing between the . and the o.

@parkr

parkr Dec 29, 2016

Member

:title.:output_ext here – there is a colon missing between the . and the o.

This comment has been minimized.

@tomjoht

tomjoht Dec 29, 2016

Contributor

updated

@tomjoht

tomjoht Dec 29, 2016

Contributor

updated

Show outdated Hide outdated docs/_docs/collections.md
## Liquid Attributes
### Collections
Each collection is accessible via the `site` Liquid variable. For example, if
Each collection is accessible through the `site` variable. For example, if

This comment has been minimized.

@parkr

parkr Dec 29, 2016

Member

I'm fine with either. Latinates will be very understandable to folks who speak Latin-derived languages, but may prove a barrier for many others. Perhaps removing this sentence altogether and replacing it with:

Each collection is accessible as a field on the site Liquid variable.

@parkr

parkr Dec 29, 2016

Member

I'm fine with either. Latinates will be very understandable to folks who speak Latin-derived languages, but may prove a barrier for many others. Perhaps removing this sentence altogether and replacing it with:

Each collection is accessible as a field on the site Liquid variable.

@tomjoht

This comment has been minimized.

Show comment
Hide comment
@tomjoht

tomjoht Dec 29, 2016

Contributor

I made the updates here.

Contributor

tomjoht commented Dec 29, 2016

I made the updates here.

@DirtyF

DirtyF approved these changes Jan 8, 2017

@tomjoht

This comment has been minimized.

Show comment
Hide comment
@tomjoht

tomjoht Jan 15, 2017

Contributor

Just checking, are you waiting on me for anything related to this PR? I'd like to see all my PRs either get merged or closed, rather than go stale.

Contributor

tomjoht commented Jan 15, 2017

Just checking, are you waiting on me for anything related to this PR? I'd like to see all my PRs either get merged or closed, rather than go stale.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli
Member

ashmaroli commented Jan 15, 2017

/cc @parkr

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 17, 2017

Member

@jekyllbot: merge +site

Member

parkr commented Jan 17, 2017

@jekyllbot: merge +site

@jekyllbot jekyllbot merged commit 16d71b1 into jekyll:master Jan 17, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Jan 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment