Skip to content

Conversation

@ggggggggg
Copy link
Contributor

@ggggggggg ggggggggg commented Oct 17, 2017

I was confused by the Timer docstrings, so I tried to make them more clear. I didn't add a doc test because it seems wasteful to have a test that is mostly sleeping. I don't have a local build of julia to build the docs with, so I just tried to get the formatting right.

Normally I would have re-used most of the text and just made a small entry for the callback variant, but one docstring is on a type, and one is on a method, so I just repeated everything.

wait(t)
sleep(.5)
close(t)
end
Copy link
Member

Choose a reason for hiding this comment

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

Typically we use the preferred style even in REPL examples. So this should be written:

julia> begin
           i = 0
           cb(timer) = (global i += 1; println(i))
           t = Timer(cb, 2, 0.2)
           wait(t)
           sleep(0.5)
           close(t)
       end

@ararslan ararslan added the docs This change adds or pertains to documentation label Oct 17, 2017
@fredrikekre
Copy link
Member

Please don't ci skip unless (at least) running make check-whitespace locally. Here there was a trailing space on (almost) every line, which fails the build.

@ararslan
Copy link
Member

For doc changes it's also a good idea to build the docs locally to ensure they render how you expect them to.

@ggggggggg
Copy link
Contributor Author

I'm a bit confused about the expectations on documentation PRs. One the one hand, CONTRIBUTING.md says to build locally, and I've been asked to do so. On the other hand, when you browse the docs there is a "Edit on Github" link, and a "Source" link next to every method entry. Those suggest that clicking on them, editing, and submitting a PR might be helpful. It seems like a mixed message.

Thanks for the corrections.

Can I run make docs with a standard julia install, or do I need to build from source?

@fredrikekre
Copy link
Member

Editing on github can be conveninent for minor changes, but for larger changes I would recommend doing the work locally. It is fine to do the changes on github if you prefer.

@fredrikekre fredrikekre merged commit d810e08 into JuliaLang:master Oct 18, 2017
@StefanKarpinski
Copy link
Member

We could also turn off whitespace failures in favor of automatically fixing them (e.g. with a bot), which I've been in favor of since it's much friendlier to people making changes, e.g. from the web.

@ararslan
Copy link
Member

It would also be nice for it to correct things like indentation.

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

Labels

docs This change adds or pertains to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants