Fix some v2.0.0 docs #82

Kyle-Verhoog · 2018-05-17T14:01:55Z

Building (and reading through) the documentation revealed a number of issues:

...opentracing.Tracer.start_active_span:8: WARNING: Definition list ends without a blank line; unexpected unindent.
...opentracing.Tracer.start_active_span:16: WARNING: Block quote ends without a blank line; unexpected unindent.
...opentracing.Tracer.start_active_span:18: WARNING: Definition list ends without a blank line; unexpected unindent.
../CHANGELOG.rst:30: WARNING: Inline strong start-string without end-string.
../CHANGELOG.rst:118: WARNING: Bullet list ends without a blank line; unexpected unindent.
...
copying static files... WARNING: html_static_path entry u'/Users/kyle.verhoog/dev/ot-py-kyle/docs/_static' does not exist
...
build succeeded, 6 warnings.

This PR fixes these warnings.

This PR also attempts to:

Use :meth:, :attr:, :class: and :exc: where possible to make navigating the docs much easier.
Convert mentions of instances of classes to lower case
Use single backticks for argument names
Use double backticks for inline code and operators
Add argument types

@palazzem

palazzem · 2018-05-18T13:09:39Z

@carlosalberto it's mostly cosmetic and fixing the warnings. I think it can go in before having conflicts with newer PRs?

carlosalberto · 2018-05-19T14:21:47Z

opentracing/tracer.py

@@ -108,7 +108,7 @@ def start_active_span(self,
        :param finish_on_close: whether span should automatically be finished
            when `Scope#close()` is called.

-         :return: a `Scope`, already registered via the `ScopeManager`.
+        :return: a `Scope`, already registered via the `ScopeManager`.


Double backquotes maybe?

carlosalberto · 2018-05-19T14:22:02Z

opentracing/tracer.py

@@ -57,7 +57,7 @@ def active_span(self):
        Tracer.scope_manager.active.span, and None will be returned if
        Scope.span is None.

-        :return: returns the active Span.
+        :return: the active Span.


Backquote(s) here maybe, too?

carlosalberto · 2018-05-19T14:23:23Z

@palazzem Oh, definitely.

@Kyle-Verhoog thanks for the patch. Left a pair of comments for you ;) Thank you so much for jumping in with the docs effort!

Kyle-Verhoog · 2018-05-21T21:56:03Z

@carlosalberto this should be good to go 😄

carlosalberto · 2018-05-22T17:46:24Z

Hey @Kyle-Verhoog thanks a lot for the iteration ;)

I was wondering, as a general doubt/idea, whether we should use :class: for Span and other members (and the same for functions).

I say this because, upon rendering your effort (through make docs), I realized that Span and ScopeManager.activate() show up using different styles, which can look a little bit strange (I went and check other Python libraries and saw they use the same style for classes/functions/methods/properties).

What do you know? Do you have an opinion on this? Thanks a lot and let me know ;)

Kyle-Verhoog · 2018-05-22T21:35:03Z

@carlosalberto yeah it crossed my mind as I was making some of the fixes (whether or not to just do all of the formatting). I decided just to fix up the ones I came across, but maybe this is a good opportunity to just format all the things?

I think it might be overkill to format all the Span references unless it is referring to the actual class specifically then we could use :class:. I see in the Sphinx documentation that they don't format every occurrence of a class or instance of a class.

I personally don't have any strong opinions as long as things are consistent (which they currently aren't). 😄

I can go ahead and format all the things if we want to do that in this PR.

carlosalberto · 2018-05-23T02:37:21Z

Hey @Kyle-Verhoog Thanks for the answer. I'd go for the formatting in case you have time - else, we can merge this PR (as this is definitely an improvement). Le me know ;)

cc @yurishkuro

- use :meth: :class: :attr: and :exc: where possible - convert mentions of instances of classes to lowercase - add argument types - use double backtick for inline code and operators

Kyle-Verhoog · 2018-05-23T21:47:30Z

@carlosalberto I went ahead and tried to make everything consistent. Let me know what you think.

carlosalberto · 2018-05-30T20:57:47Z

@Kyle-Verhoog I have to say I didn't expect to have 'Span' and other instances changed from the current form to 'span' and similar (I had expected ':class:Span', at least to separate the style).

Maybe @yurishkuro can give his opinion on this? ;)

carlosalberto · 2018-06-06T22:19:10Z

@palazzem Hey, what's your take on this, btw? Hopefully we can get this going to get closer to the actual release ;)

carlosalberto · 2018-06-08T18:24:33Z

@Kyle-Verhoog sorry for the late feedback - busy times here...

Anyway, I think the best would be merge your changes, but without the last commit - sorry for making you lose time on that last iteration :(

Let us know and we will merge this PR prior to the long delayed release ;)

Kyle-Verhoog · 2018-06-08T18:50:19Z

@carlosalberto sure, whatever you think works best.

To make the case for my changes: I found that, for example, "Span" / "Span" to just be a bit too cumbersome and out of place to both read and mark-up whenever referring to a span instance. Especially with the type annotations in place, it's usually quite easy to navigate to the doc section for the class for an instance.

Eg:

But I understand if this opinion is not shared. In hindsight it would have been good to break off those change into another commit.. 😛

carlosalberto · 2018-06-08T19:11:35Z

Hey @Kyle-Verhoog thanks for the explanation.

So I was trying to follow the pattern I had seen in requests: http://docs.python-requests.org/en/master/api/#main-interface

But I'd say we don't need to have a link per each Span and others though ;)

Kyle-Verhoog · 2018-06-08T20:00:59Z

@carlosalberto ah, I see. I can revert my changes on the lower-casing and follow a similar style to the requests library.

I'd rather see some of the work in that last commit kept since a bunch of type annotations were also added, along with some other improvements.

I'm fine with whatever you decide. Let me know if I should go ahead with those changes, I can get those done today still! 😄

carlosalberto · 2018-06-11T09:44:33Z

Hey @Kyle-Verhoog sorry for not answering before.

I can revert my changes on the lower-casing and follow a similar style to the requests library.

Yes, if you have some time.

I'd rather see some of the work in that last commit kept since a bunch of type annotations were also added, along with some other improvements.

Oh, definitely ;)

Kyle-Verhoog · 2018-06-11T20:13:51Z

@carlosalberto let me know what you think!

carlosalberto · 2018-06-12T18:49:02Z

Oh, lovely, thanks a lot @Kyle-Verhoog !

Unless @yurishkuro disagrees, I think we should merge this soon ;)

yurishkuro · 2018-06-12T18:58:25Z

palazzem · 2018-06-13T08:58:02Z

@carlosalberto all good from my side! Thank you very much @Kyle-Verhoog!

carlosalberto · 2018-06-14T15:33:24Z

Merged, thanks ;)

Kyle-Verhoog added 2 commits May 16, 2018 18:12

Fix some docs

0c354ff

Lint doc line lengths

4de0791

palazzem requested a review from carlosalberto May 18, 2018 13:08

carlosalberto reviewed May 19, 2018

View reviewed changes

Correct a few more lines

a642acf

More formatting

413bb6b

- use :meth: :class: :attr: and :exc: where possible - convert mentions of instances of classes to lowercase - add argument types - use double backtick for inline code and operators

carlosalberto mentioned this pull request Jun 8, 2018

2.0.0 Release #65

Closed

apply a similar formatting as the requests library

3036179

palazzem approved these changes Jun 13, 2018

View reviewed changes

carlosalberto merged commit cb3eb7b into opentracing:v2.0.0 Jun 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some v2.0.0 docs #82

Fix some v2.0.0 docs #82

Kyle-Verhoog commented May 17, 2018 •

edited

Loading

palazzem commented May 18, 2018

carlosalberto May 19, 2018

carlosalberto May 19, 2018

carlosalberto commented May 19, 2018

Kyle-Verhoog commented May 21, 2018

carlosalberto commented May 22, 2018

Kyle-Verhoog commented May 22, 2018

carlosalberto commented May 23, 2018

Kyle-Verhoog commented May 23, 2018

carlosalberto commented May 30, 2018

carlosalberto commented Jun 6, 2018

carlosalberto commented Jun 8, 2018

Kyle-Verhoog commented Jun 8, 2018

carlosalberto commented Jun 8, 2018

Kyle-Verhoog commented Jun 8, 2018 •

edited

Loading

carlosalberto commented Jun 11, 2018

Kyle-Verhoog commented Jun 11, 2018

carlosalberto commented Jun 12, 2018

yurishkuro commented Jun 12, 2018

palazzem commented Jun 13, 2018

carlosalberto commented Jun 14, 2018

Fix some v2.0.0 docs #82

Fix some v2.0.0 docs #82

Conversation

Kyle-Verhoog commented May 17, 2018 • edited Loading

palazzem commented May 18, 2018

carlosalberto May 19, 2018

Choose a reason for hiding this comment

carlosalberto May 19, 2018

Choose a reason for hiding this comment

carlosalberto commented May 19, 2018

Kyle-Verhoog commented May 21, 2018

carlosalberto commented May 22, 2018

Kyle-Verhoog commented May 22, 2018

carlosalberto commented May 23, 2018

Kyle-Verhoog commented May 23, 2018

carlosalberto commented May 30, 2018

carlosalberto commented Jun 6, 2018

carlosalberto commented Jun 8, 2018

Kyle-Verhoog commented Jun 8, 2018

carlosalberto commented Jun 8, 2018

Kyle-Verhoog commented Jun 8, 2018 • edited Loading

carlosalberto commented Jun 11, 2018

Kyle-Verhoog commented Jun 11, 2018

carlosalberto commented Jun 12, 2018

yurishkuro commented Jun 12, 2018

palazzem commented Jun 13, 2018

carlosalberto commented Jun 14, 2018

Kyle-Verhoog commented May 17, 2018 •

edited

Loading

Kyle-Verhoog commented Jun 8, 2018 •

edited

Loading