openwisp / openwisp-utils

Python and Django utilities shared between different openwisp modules
https://openwisp.io/docs/dev/utils/
BSD 3-Clause "New" or "Revised" License
74 stars 72 forks source link

[qa] ReStructuredCheck failing in netjsonconfig, different approach is needed #150

Closed nemesifier closed 4 years ago

nemesifier commented 4 years ago

The check is failing with:

There are some Errors in your RST file syntax 

- <string>:1: (WARNING/2) "raw" directive disabled.

However, I don't think the path we've taken is good.

The repositories which have sphinx documentation already have checks to ensure that documentation compiles and renders successfully, these checks simply generate the sphinx docs and they work reliably.

Now we're mixing things. I asked to add this check because we need to check that the README files are compatible with pypi. If we start allowing sphinx syntax in the README, we'll hit again the same issue that some of the sphinx syntax won't work on pypi.

I think the right solution is to separate the two things.

Sphinx documentation checks must be done as we already do: https://github.com/openwisp/netjsonconfig/blob/master/run-qa-checks#L8-L10 This new check should only focus on the README compatibility with pypi, removing the changes we introduced recently to make it work on openwisp-radius. I would also rename the check to make this more explicit: we can call it checkreadme to ensure things don't get confused.

@KapilBansal320 what do you think?

devkapilbansal commented 4 years ago

@nemesisdesign I think there can be two ways here.

  1. Muting the warnings here which doesn't seems feasible.
  2. Following your approach to check readme only and stop recursive checking on all ReStructuredText files.
nemesifier commented 4 years ago

@nemesisdesign I think there can be two ways here.

  1. Muting the warnings here which doesn't seems feasible.
  2. Following your approach to check readme only and stop recursive checking on all ReStructuredText files.

I'd go for 2 because we need warnings, pypi does not support many things used in sphinx so raising the error is the right thing to do, but only when checking the README. Sphinx docs have their own check in each repo.

devkapilbansal commented 4 years ago

Okay. But I am thinking that Shouldn't we change the raw directives to image.

nemesifier commented 4 years ago

Okay. But I am thinking that Shouldn't we change the raw directives to image.

@KapilBansal320 the raw is for HTML iframes.