Skip to content

Conversation

@felixdivo
Copy link
Collaborator

@felixdivo felixdivo commented Jul 1, 2018

@hardbyte I'll fix some errors here based on the work you have already done a few months back, and notify you when I'm done.

@felixdivo felixdivo changed the title Fix Sphinx errors/warnings Fix Sphinx errors/warnings & Misc. cleanups Jul 1, 2018
@felixdivo felixdivo added this to the 2.3 Release milestone Jul 1, 2018
@felixdivo
Copy link
Collaborator Author

I honestly can't make much sense of the remaining Sphinx output, like this one.

@codecov
Copy link

codecov bot commented Jul 1, 2018

Codecov Report

❗ No coverage uploaded for pull request base (develop@0b7de38). Click here to learn what that means.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #347   +/-   ##
==========================================
  Coverage           ?   58.83%           
==========================================
  Files              ?       54           
  Lines              ?     4273           
  Branches           ?        0           
==========================================
  Hits               ?     2514           
  Misses             ?     1759           
  Partials           ?        0
Impacted Files Coverage Δ
can/bus.py 79.26% <ø> (ø)
can/interfaces/pcan/basic.py 71.89% <ø> (ø)
can/broadcastmanager.py 39.34% <ø> (ø)
can/interfaces/iscan.py 36.61% <100%> (ø)
can/interfaces/pcan/pcan.py 28.08% <100%> (ø)
can/util.py 71.01% <50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b7de38...dd7c0d3. Read the comment docs.

@hardbyte
Copy link
Owner

hardbyte commented Jul 2, 2018

Yeah I find it quite hard to track down what sphinx complains about. Most of those reference target not found messages are because we refer to something that isn't included in the docs (often because the method has not docstring, or because we haven't included the particular api docs in a sphinx page).

can/util.py Outdated
return config


def choose_socketcan_implementation():
Copy link
Owner

Choose a reason for hiding this comment

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

This shouldn't be in a doc update PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This branch that you created quite a while back was a few dozen commits behind develop, and this resulted from the rebase to resolve the conflicts. Was this wrong?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh right I didn't notice you were starting with an older branch. Yes this was removed on purpose (in the socketCAN consolidation).

I suggest rebasing on top of develop - either force push over this branch or to another branch.

Copy link
Owner

Choose a reason for hiding this comment

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

If it helps I've rebased my original clean up commits on the current develop branch fix-sphinx-errors-rebase

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Uhm, I already applied other patches on top of the rebase. Wouldn't it be the simplest to just remove the choose_socketcan_implementation() function?

Copy link
Owner

Choose a reason for hiding this comment

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

Sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@felixdivo
Copy link
Collaborator Author

@hardbyte
Copy link
Owner

hardbyte commented Jul 3, 2018

No I don't think there is a problem with sphinx. I think we just haven't been particularly careful with our docs.

One additional thing though - we should check the heading levels make sense. In the 2.2.0 release we accidentally included this https://python-can.readthedocs.io/en/2.2.0/development.html#about-the-busabc-class

@felixdivo
Copy link
Collaborator Author

Okay; I was rather supposing that we ask for how to get more helpful error statements out of Sphinx' build process.

Well, was that section really an accident? It is in the Developer's section, which is the right place for that, isn't it?

@hardbyte
Copy link
Owner

hardbyte commented Jul 3, 2018

Sorry I should have been clearer - the section's content is fine, it is the heading level that is making it much more important than it should be:

TOC

@felixdivo
Copy link
Collaborator Author

Oh sure, that's not meant to be!

@felixdivo
Copy link
Collaborator Author

Does not fix all problems, so #262 remains open.

@felixdivo felixdivo merged commit 6d0f4a6 into develop Jul 5, 2018
@felixdivo felixdivo deleted the fix-sphinx-errors branch July 5, 2018 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants