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

Audit stdlib documentation #1196

Merged
merged 3 commits into from Mar 31, 2023
Merged

Audit stdlib documentation #1196

merged 3 commits into from Mar 31, 2023

Conversation

vkleen
Copy link
Member

@vkleen vkleen commented Mar 23, 2023

For now, a quick pass over the stdlib documentation for consistency and typos.

@github-actions github-actions bot temporarily deployed to pull request March 23, 2023 13:38 Inactive
@github-actions github-actions bot temporarily deployed to pull request March 23, 2023 14:05 Inactive
@vkleen vkleen marked this pull request as ready for review March 27, 2023 09:25
@github-actions github-actions bot temporarily deployed to pull request March 27, 2023 09:27 Inactive
Copy link
Member

@yannham yannham left a comment

Choose a reason for hiding this comment

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

So I had to stop at some point with suggestions, there were just too many lines 😓 but I think we should be consistent over:

  • verbal forms. If we start some documentation with Returns, then all verbs describing the action of a function should use the s form.
  • indentation. I believe there are cases where the code is indented with respect to the block fences, but not everywhere. Also, I don't think it's standard practice, it feels like it should start the the same level as the block, as I don't see much usefulness in having the whole code block indented by one level from the leftmost boundary.

stdlib/array.ncl Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/function.ncl Outdated Show resolved Hide resolved
stdlib/function.ncl Outdated Show resolved Hide resolved
stdlib/number.ncl Outdated Show resolved Hide resolved
stdlib/number.ncl Outdated Show resolved Hide resolved
stdlib/number.ncl Outdated Show resolved Hide resolved
@vkleen
Copy link
Member Author

vkleen commented Mar 28, 2023

So I had to stop at some point with suggestions, there were just too many lines

No worries 😆 I'll find all the lines.

* verbal forms. If we start some documentation with `Returns`, then all verbs describing the action of a function should use the `s` form.

👍

* indentation. I believe there are cases where the code is indented with respect to the block fences, but not everywhere. Also, I don't think it's standard practice, it feels like it should start the the same level as the block, as I don't see much usefulness in having the whole code block indented by one level from the leftmost boundary.

It always felt to me like a fenced in block acts like a new scope, and as such deserves to be indentend. But that may indeed just be me.

I'll make another pass over all of it, remove indents from code block and pay attention to the verbal forms.

@yannham
Copy link
Member

yannham commented Mar 28, 2023

Sorry if my suggestions sounded a bit too affirmative 😅 I'm not necessarily pushing for the trailing s, and maybe there's a case to be made for indenting code inside a fenced block? Let met quickly check if I can find style advises online. But the hard constraint is that we're consistent.

@yannham
Copy link
Member

yannham commented Mar 28, 2023

So with respect to indenting code, it seems like GitHub's documentation and W3C's documentation show examples where the code isn't indented indeed (starts at the same indentation as the fence).

@vkleen
Copy link
Member Author

vkleen commented Mar 28, 2023

No worries 😺 As you can tell from me preferring the trailing "s" on one day and preferring the other form on the next, I don't have a stable preference on the matter. But with you we have a slight bias towards trailing "s", so I'm taking that.

@github-actions github-actions bot temporarily deployed to pull request March 28, 2023 09:13 Inactive
@vkleen vkleen requested a review from yannham March 29, 2023 11:29
@vkleen vkleen force-pushed the stdlib-docs-pass branch 2 times, most recently from 9ac616e to 897e9e4 Compare March 30, 2023 22:39
@github-actions github-actions bot temporarily deployed to pull request March 30, 2023 22:43 Inactive
@yannham
Copy link
Member

yannham commented Mar 31, 2023

Oh my, I've merged #1198, which might make this rebase quite difficult. Might be simpler in the end to revert, merge this one, and reformat afterward? Or do you think it'll be manageable?

stdlib/builtin.ncl Outdated Show resolved Hide resolved
stdlib/function.ncl Outdated Show resolved Hide resolved
stdlib/function.ncl Outdated Show resolved Hide resolved
stdlib/record.ncl Show resolved Hide resolved
stdlib/string.ncl Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request March 31, 2023 14:21 Inactive
Consistently use markdown heading for examples

Consistently use third-person in stdlib docs

Adjust documentation string indentation in stdlib

Contract to enforce -> Enforces

NumLiteral -> NumberLiteral
@github-actions github-actions bot temporarily deployed to pull request March 31, 2023 14:45 Inactive
@vkleen vkleen merged commit f0854b9 into master Mar 31, 2023
4 checks passed
@vkleen vkleen deleted the stdlib-docs-pass branch March 31, 2023 15:11
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.

None yet

2 participants