New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix] Fixed error in ReStructuredText Check #150 #151
Conversation
@nemesisdesign please review it. Here I removed recursive checking on the subdirectories like docs as said. Also, ignore_sphinx is removed here. Although I don't rename checkrst to checkreadme as I was checking other rst files in root directory too. Like CHANGES.rst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @KapilBansal320, looks good, but there's an issue, see my comment below and let me know if you can do that change.
@@ -147,7 +144,7 @@ def receive_url(self, obj): | |||
) | |||
baseurl = self.receive_url_baseurl | |||
if not baseurl: | |||
baseurl = '{0}://{1}'.format(self.request.scheme, self.request.get_host(),) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's many changes like this one which are not related to the goal of the PR, this makes history hard to follow.
Can you please separate these changes into a separate commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nemesisdesign I break down my commit in two, one for the changes and second for the formatting. Is it correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, thanks
Travis tests have failedHey @KapilBansal320, 6th Build./run-qa-checks
TravisBuddy Request Identifier: 5255d810-fe39-11ea-803e-573615702161 |
Travis tests have failedHey @KapilBansal320, 6th Build./run-qa-checks
TravisBuddy Request Identifier: c6acbeb0-fe50-11ea-803e-573615702161 |
@nemesisdesign I am changing the function to check only for readme instead because when I ran it on openwisp2-docs, it fails for index.rst file (as toctree is a sphinx directive and docutils doesn't recognize it). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work @devkapilbansal, thanks 👍
@@ -147,7 +144,7 @@ def receive_url(self, obj): | |||
) | |||
baseurl = self.receive_url_baseurl | |||
if not baseurl: | |||
baseurl = '{0}://{1}'.format(self.request.scheme, self.request.get_host(),) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, thanks
Fixes #150