Convert documentation from epydoc to reStructuredText #387

JoeLametta · 2019-03-21T14:13:21Z

No description provided.

Freso

Looks mostly okay so far. I had some other thoughts about the overall thing, but I think I lost those about half-way down the page of code changes…

whipper/common/accurip.py

whipper/common/cache.py

whipper/common/common.py

whipper/common/program.py

JoeLametta · 2019-03-22T08:10:16Z

I've slightly reworded/abbreviated the docstrings because the first line should be a summary line (on its own) and fit in the char limit of PEP 8 (I think for docstrings the limit is even stricter to 72 chars but I'm not going to apply it).

JoeLametta · 2019-03-22T08:13:00Z

Because of sphinx.ext.autodoc this is related to #9.

Freso · 2019-03-22T10:38:03Z

I've slightly reworded/abbreviated the docstrings because the first line should be a summary line (on its own) and fit in the char limit of PEP 8

Which is great and something that should absolutely be done, but it's not really related to/part of converting epydoc to rST.

JoeLametta · 2019-03-22T15:50:58Z

@Freso Force pushed. This time I think I've committed only the appropriate changes. 🙏

JoeLametta · 2019-05-03T10:29:37Z

@Freso Ping. 😉

Freso

No longer any complaints, I think. It’s pretty much a straight up 1:1 port of epydoc → (Sphinx flavoured) reStructuredText.

One comment maybe. A lot of L{…} were changed to just …. Maybe it’d make sense to use the :any: role/directive for these? Like :any:`…`. Either way, that’s not something to hold this PR up for.

JoeLametta · 2019-05-03T13:40:10Z

One comment maybe. A lot of L{…} were changed to just …. Maybe it’d make sense to use the :any: role/directive for these? Like :any:`…`. Either way, that’s not something to hold this PR up for.

Done!
To you does it make sense to squash the latest commit into the previous one?

Freso

Aaand… some small changes/fixups needed now. :)

I do think it’s probably fine to squash all the :any: related changes to a single commit.

whipper/common/program.py

whipper/common/common.py

whipper/common/mbngs.py

whipper/common/program.py

whipper/result/result.py

Freso · 2019-05-03T14:10:35Z

whipper/common/mbngs.py

+    :type  release:      unicode
+    :param title:        title of the disc (with disambiguation)
+    :param releaseTitle: title of the release (without disambiguation)
+    :type  tracks:       list of :any:`TrackMetadata`


Suggested change

:type tracks: list of :any:`TrackMetadata`

:type tracks: :class:`list` of :any:`TrackMetadata`

I think I'm going to use this:

:type tracks: list(:class:`TrackMetadata`)

My suggestion was for an as 1:1 conversion as possible. Do you have a link to documentation for how list(SomeClass) is interpreted by Sphinx?

https://www.pydev.org/manual_adv_type_hints.html

If it's out of scope, I'll add these (and similar changes in #389) 😄

I wouldn’t say they’re out‐of‐scope for this PR—it’s still converting epydoc format to reST/Sphinx format.

Also not sure what https://www.pydev.org/manual_adv_type_hints.html is, but it’s not upstream Sphinx docs at least. Trying to see if I can find this documented in Sphinx’s docs too.

I asked about it on IRC in #sphinx-doc, but maybe just leave it out for now? If we either determine that there’s an official reST/Sphinx way we can do that, or we can make another PR to make it conform to the Eclipse way of doing things. Seems like it’s a reach for the scope of this PR which is so close now, so it also seems like this is such a small thing to hold the merging of the PR back on the basis of.

whipper/result/result.py

JoeLametta · 2019-05-03T14:22:14Z

I do think it’s probably fine to squash all the :any: related changes to a single commit.

You mean a separate commit from 44680cd, right?

Freso · 2019-05-03T14:26:38Z

Eh. I’d actually think it’d be fine to squash all of the commits here into a single one. It’s relatively small and self‐contained change, pretty much a 1:1 conversion of epydoc to (Sphinx‐style) reST. This is probably one of the few times where I’ll say it’s probably overkill to keep them as 3+ separate commits. :)

Freso

If we’re changing the list of Foo to list(Foo), let’s be consistent. This is one thing I’d say should maybe be its own commit though, because it deviates a teeny tiny bit from a straight 1:1 conversion—but if the syntax is legit, then I think it’s fine to keep it as part of this PR.

(Also, after pointing out a number of list of and dict ofs, I just stopped. grep yourself to find all of them. :))

whipper/common/program.py

whipper/image/cue.py

whipper/image/image.py

whipper/image/table.py

whipper/program/cdparanoia.py

whipper/common/common.py

whipper/image/table.py

Freso

A couple of minor things left unresolved, but eh, it’s fine to go in now, I’d say. It’ll need a lot of polish after getting in anyway, so we can clean up things in follow‐up PRs and commits.

Freso · 2019-05-03T17:46:17Z

whipper/program/cdparanoia.py

@@ -205,8 +205,6 @@ def getTrackQuality(self):
 class ReadTrackTask(task.Task):
    """
    I am a task that reads a track using cdparanoia.
-
-    @ivar reads: how many reads were done to rip the track


Is there a reason this is removed instead of replaced? Is it because it’s referring to something that’s been removed? I’m slightly leaning towards converting it and then removing it in #389. But eh.

Is it because it’s referring to something that’s been removed?

That's it.

whipper/result/result.py

Thanks to Freso for all the useful comments! Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>

Freso · 2019-05-03T18:08:00Z

I think I’ve resolved all actually resolved remarks now. None of the remaining remarks need fixing for this PR to go in. My vote is to just get it in and move on to #389 and other endeavours that actually improve the documentation instead of simply converting it.

JoeLametta · 2019-05-03T18:11:25Z

Merged! 🎆

Freso reviewed Mar 22, 2019

View reviewed changes

JoeLametta force-pushed the feature/issue-383-restructuredtext branch from 4c94f7b to d630c40 Compare March 22, 2019 07:57

JoeLametta force-pushed the feature/issue-383-restructuredtext branch from d630c40 to bfdbfcb Compare March 22, 2019 14:30

JoeLametta mentioned this pull request Mar 22, 2019

Improve docstrings #389

Merged

JoeLametta force-pushed the feature/issue-383-restructuredtext branch 2 times, most recently from 1b16fc2 to 44680cd Compare March 29, 2019 12:36

JoeLametta force-pushed the develop branch from 58c96be to bb6a29c Compare May 3, 2019 09:44

Freso approved these changes May 3, 2019

View reviewed changes

JoeLametta force-pushed the feature/issue-383-restructuredtext branch from eeb4c94 to 83b2543 Compare May 3, 2019 13:37

JoeLametta changed the title ~~WIP: Change documentation from epydoc to reStructuredText~~ Change documentation from epydoc to reStructuredText May 3, 2019

JoeLametta changed the title ~~Change documentation from epydoc to reStructuredText~~ Convert documentation from epydoc to reStructuredText May 3, 2019

Freso requested changes May 3, 2019

View reviewed changes

JoeLametta force-pushed the feature/issue-383-restructuredtext branch from 835adc3 to 9274cd2 Compare May 3, 2019 15:51

JoeLametta force-pushed the feature/issue-383-restructuredtext branch from 9274cd2 to 0bbd2fa Compare May 3, 2019 17:09

Freso approved these changes May 3, 2019

View reviewed changes

Change documentation from epydoc to reStructuredText

69f8f39

Thanks to Freso for all the useful comments! Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>

JoeLametta force-pushed the feature/issue-383-restructuredtext branch from 0bbd2fa to 69f8f39 Compare May 3, 2019 18:06

JoeLametta merged commit bb78dc1 into develop May 3, 2019

JoeLametta deleted the feature/issue-383-restructuredtext branch May 3, 2019 18:11

JoeLametta mentioned this pull request Sep 18, 2020

Change documentation from epydoc to reStructuredText #383

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert documentation from epydoc to reStructuredText #387

Convert documentation from epydoc to reStructuredText #387

JoeLametta commented Mar 21, 2019

Freso left a comment

JoeLametta commented Mar 22, 2019

JoeLametta commented Mar 22, 2019

Freso commented Mar 22, 2019

JoeLametta commented Mar 22, 2019

JoeLametta commented May 3, 2019

Freso left a comment

JoeLametta commented May 3, 2019

Freso left a comment

Freso May 3, 2019

JoeLametta May 3, 2019 •

edited

Loading

Freso May 3, 2019

JoeLametta May 3, 2019

JoeLametta May 3, 2019

Freso May 3, 2019

Freso May 3, 2019

Freso May 3, 2019

JoeLametta commented May 3, 2019

Freso commented May 3, 2019

Freso left a comment

Freso left a comment

Freso May 3, 2019

JoeLametta May 3, 2019

Freso commented May 3, 2019

JoeLametta commented May 3, 2019

	:type tracks: list of :any:`TrackMetadata`
	:type tracks: :class:`list` of :any:`TrackMetadata`

Convert documentation from epydoc to reStructuredText #387

Convert documentation from epydoc to reStructuredText #387

Conversation

JoeLametta commented Mar 21, 2019

Freso left a comment

Choose a reason for hiding this comment

JoeLametta commented Mar 22, 2019

JoeLametta commented Mar 22, 2019

Freso commented Mar 22, 2019

JoeLametta commented Mar 22, 2019

JoeLametta commented May 3, 2019

Freso left a comment

Choose a reason for hiding this comment

JoeLametta commented May 3, 2019

Freso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeLametta May 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeLametta commented May 3, 2019

Freso commented May 3, 2019

Freso left a comment

Choose a reason for hiding this comment

Freso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Freso commented May 3, 2019

JoeLametta commented May 3, 2019

JoeLametta May 3, 2019 •

edited

Loading