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

Explained the proc \"pretty\" in detail, file: json.nim with comments and sample program #10466

Merged
merged 4 commits into from Jan 30, 2019

Conversation

Projects
None yet
3 participants
@MandeepSinghey
Copy link
Contributor

commented Jan 26, 2019

Explained the proc "pretty" in detail, file: json.nim with comments and sample program

MandeepSinghey and others added some commits Jan 26, 2019

Merge pull request #1 from nim-lang/devel
Updated httpClient.nim: added import httpClient statements in examples

@MandeepSinghey MandeepSinghey referenced this pull request Jan 27, 2019

Closed

[help wanted] Better stdlib documentation #10330

20 of 24 tasks complete
@narimiran

This comment has been minimized.

Copy link
Member

commented Jan 27, 2019

There are several problems with this:

  1. It doesn't even compile when you do nim doc lib/pure/json.nim (backticks around $ in the example seem to be a problem)
  2. When you fix that, the created document doesn't look right. (There has to be an empty line before .. code-block::)
  3. After that, it looks like you would want, but the example is too long and verbose. (Some comments are really unnecessary)

Btw, Nim is not an acronym. It is written 'Nim', not 'NIM'. (But you should remove that "we can use in NIM as following" sentence anyways ;))


I applaud your will to help with the documentation, but please look at some other files to see how other people had written the documentation so far and try to mimic that style.

@narimiran narimiran self-assigned this Jan 27, 2019

Mandeep Singh Mandeep Singh
Added a line before code block and used double ticks before $ sign in…
… code example as per narimiran feedback
@MandeepSinghey

This comment has been minimized.

Copy link
Contributor Author

commented Jan 27, 2019

Thanks as suggested fixed all the errors.

I kept intentionally the example long, because this is what I like about python, that they have very clear end to end examples in their documentation.

I see this missing in Nim, that is why it is not getting the attention it should be as compared to Google GO.

@Araq Araq requested a review from narimiran Jan 29, 2019

simplify the example
The example is now shorter, clearer, and self-contained.

@narimiran narimiran merged commit 7118e1c into nim-lang:devel Jan 30, 2019

2 checks passed

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

timotheecour added a commit to timotheecour/Nim that referenced this pull request Jan 31, 2019

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

@MandeepSinghey thanks for the PR and looking forward for more contributions!

next time could you please use runnableExamples so that it's guaranteed to stay in sync (avoids regressions)?
see https://nim-lang.github.io/Nim/contributing.html for more details
i fixed it for you here: #10510 ; it's also more compact and more precise as you can see

narimiran added a commit that referenced this pull request Jan 31, 2019

@narimiran

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

@timotheecour we're still waiting for your contribution to #10330 — since you have the time to nit-pick around other people's PRs, it would be more beneficial to all of us if you could use that time more productively and write some documentation (correct straight from the beginning) on your own.

And if writing the documentation is too boring, here's an always-growing list of issues that need fixing.

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

and what do you think that growing list of merged prs is? cmon. documentation isnt' the only thing that needs fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.