| Message ID | 1447381120-29733-1-git-send-email-stephen.finucane@intel.com |
|---|---|
| State | RFC |
| Headers | show |
> This provides a quick, easy way to use the 'mdl' executable provided > in markdowlinter to validate documentation. > > This change does not resolve any of the issues this linter raises - > these will need to be done in a follow up patch. > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> This is strictly an RFC. I'm interested in the idea of automating validation of documentation. While we can't validate the content itself, we can certainly extract meaning from the docs. For example: 1. do something like so: echo "hello, world" 2. run something else We should be able to figure out that this is code, and that it's not been properly indented. How we do that (this tool or otherwise) is up for debate. Cheers, Stephen
On Thu, Nov 12, 2015 at 9:18 PM, Stephen Finucane < stephen.finucane@intel.com> wrote: > This provides a quick, easy way to use the 'mdl' executable provided > in markdowlinter to validate documentation. > > This change does not resolve any of the issues this linter raises - > these will need to be done in a follow up patch. > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > --- > INSTALL.md | 5 +++++ > Makefile.am | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/INSTALL.md b/INSTALL.md > index 906825a..7311915 100644 > --- a/INSTALL.md > +++ b/INSTALL.md > @@ -111,6 +111,11 @@ To run the unit tests, you also need: > - Perl. Version 5.10.1 is known to work. Earlier versions should > also work. > > +If you are going to modify Open vSwitch documentation, please consider > +installing the following to validate your changes: > + > + - "markdownlint" (https://github.com/mivok/markdownlint) > + > The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in > formats other than plain text, only if you have the following: > > diff --git a/Makefile.am b/Makefile.am > index 966ba27..d58cb59 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -377,6 +377,10 @@ dist-docs: > VERSION=$(VERSION) $(srcdir)/build-aux/dist-docs $(srcdir) $(docs) > .PHONY: dist-docs > > +lint-docs: > + mdl $(docs) > +.PHONY: dist-docs You have a typo here: s/dist-docs/lint-docs/ What results does this come up with for the current docs? That'd probably help demonstrate the value. I got errors trying to install mdl and decided to just ask for now instead. :-) In general though, I think this seems like a fine idea.
On 13 Nov 14:08, Russell Bryant wrote: > On Thu, Nov 12, 2015 at 9:18 PM, Stephen Finucane <stephen.finucane@intel.com> > wrote: > > This provides a quick, easy way to use the 'mdl' executable provided > in markdowlinter to validate documentation. > > This change does not resolve any of the issues this linter raises - > these will need to be done in a follow up patch. > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > --- > INSTALL.md | 5 +++++ > Makefile.am | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/INSTALL.md b/INSTALL.md > index 906825a..7311915 100644 > --- a/INSTALL.md > +++ b/INSTALL.md > @@ -111,6 +111,11 @@ To run the unit tests, you also need: > - Perl. Version 5.10.1 is known to work. Earlier versions should > also work. > > +If you are going to modify Open vSwitch documentation, please consider > +installing the following to validate your changes: > + > + - "markdownlint" (https://github.com/mivok/markdownlint) > + > The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in > formats other than plain text, only if you have the following: > > diff --git a/Makefile.am b/Makefile.am > index 966ba27..d58cb59 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -377,6 +377,10 @@ dist-docs: > VERSION=$(VERSION) $(srcdir)/build-aux/dist-docs $(srcdir) $(docs) > .PHONY: dist-docs > > +lint-docs: > + mdl $(docs) > +.PHONY: dist-docs > > > You have a typo here: s/dist-docs/lint-docs/ This is what happens when I write code at night :) > What results does this come up with for the current docs? That'd probably help > demonstrate the value. I got errors trying to install mdl and decided to just > ask for now instead. :-) > > In general though, I think this seems like a fine idea. Are you ready for this (starts humming the Space Jam tune)? I've dropped roughly 60% of the "issues" so I may be missing some error types, but you should get the gist. This is all configurable, so we can switch off what we don't agree with. AFAIK the tool supports custom rules also so we could add our own (locally or upstream) for commonly seen issues. Stephen
On Fri, Nov 13, 2015 at 11:40 AM, Finucane, Stephen < stephen.finucane@intel.com> wrote: > On 13 Nov 14:08, Russell Bryant wrote: > > On Thu, Nov 12, 2015 at 9:18 PM, Stephen Finucane < > stephen.finucane@intel.com> > > wrote: > > > > This provides a quick, easy way to use the 'mdl' executable provided > > in markdowlinter to validate documentation. > > > > This change does not resolve any of the issues this linter raises - > > these will need to be done in a follow up patch. > > > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > > --- > > INSTALL.md | 5 +++++ > > Makefile.am | 4 ++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/INSTALL.md b/INSTALL.md > > index 906825a..7311915 100644 > > --- a/INSTALL.md > > +++ b/INSTALL.md > > @@ -111,6 +111,11 @@ To run the unit tests, you also need: > > - Perl. Version 5.10.1 is known to work. Earlier versions should > > also work. > > > > +If you are going to modify Open vSwitch documentation, please > consider > > +installing the following to validate your changes: > > + > > + - "markdownlint" (https://github.com/mivok/markdownlint) > > + > > The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in > > formats other than plain text, only if you have the following: > > > > diff --git a/Makefile.am b/Makefile.am > > index 966ba27..d58cb59 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -377,6 +377,10 @@ dist-docs: > > VERSION=$(VERSION) $(srcdir)/build-aux/dist-docs $(srcdir) > $(docs) > > .PHONY: dist-docs > > > > +lint-docs: > > + mdl $(docs) > > +.PHONY: dist-docs > > > > > > You have a typo here: s/dist-docs/lint-docs/ > > This is what happens when I write code at night :) > > > What results does this come up with for the current docs? That'd > probably help > > demonstrate the value. I got errors trying to install mdl and decided > to just > > ask for now instead. :-) > > > > In general though, I think this seems like a fine idea. > > Are you ready for this (starts humming the Space Jam tune)? I've dropped > roughly 60% > of the "issues" so I may be missing some error types, but you should get > the gist. > This is all configurable, so we can switch off what we don't agree with. > AFAIK the > tool supports custom rules also so we could add our own (locally or > upstream) for > commonly seen issues. > Thanks for sharing. It'd be nice to strip this down to errors that will actually negatively affect the docs getting rendered properly. If it's something that breaks viewing the doc on github or breaks the HTML version in dist-docs, that's something that would be nice to catch automatically. I'm personally much less concerned about the style nits.
On 13 Nov 13:18, Russell Bryant wrote: > On Fri, Nov 13, 2015 at 11:40 AM, Finucane, Stephen < > stephen.finucane@intel.com> wrote: > > > On 13 Nov 14:08, Russell Bryant wrote: > > > On Thu, Nov 12, 2015 at 9:18 PM, Stephen Finucane < > > stephen.finucane@intel.com> > > > wrote: > > > > > > This provides a quick, easy way to use the 'mdl' executable provided > > > in markdowlinter to validate documentation. > > > > > > This change does not resolve any of the issues this linter raises - > > > these will need to be done in a follow up patch. > > > > > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > > > --- > > > INSTALL.md | 5 +++++ > > > Makefile.am | 4 ++++ > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/INSTALL.md b/INSTALL.md > > > index 906825a..7311915 100644 > > > --- a/INSTALL.md > > > +++ b/INSTALL.md > > > @@ -111,6 +111,11 @@ To run the unit tests, you also need: > > > - Perl. Version 5.10.1 is known to work. Earlier versions should > > > also work. > > > > > > +If you are going to modify Open vSwitch documentation, please > > consider > > > +installing the following to validate your changes: > > > + > > > + - "markdownlint" (https://github.com/mivok/markdownlint) > > > + > > > The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in > > > formats other than plain text, only if you have the following: > > > > > > diff --git a/Makefile.am b/Makefile.am > > > index 966ba27..d58cb59 100644 > > > --- a/Makefile.am > > > +++ b/Makefile.am > > > @@ -377,6 +377,10 @@ dist-docs: > > > VERSION=$(VERSION) $(srcdir)/build-aux/dist-docs $(srcdir) > > $(docs) > > > .PHONY: dist-docs > > > > > > +lint-docs: > > > + mdl $(docs) > > > +.PHONY: dist-docs > > > > > > > > > You have a typo here: s/dist-docs/lint-docs/ > > > > This is what happens when I write code at night :) > > > > > What results does this come up with for the current docs? That'd > > probably help > > > demonstrate the value. I got errors trying to install mdl and decided > > to just > > > ask for now instead. :-) > > > > > > In general though, I think this seems like a fine idea. > > > > Are you ready for this (starts humming the Space Jam tune)? I've dropped > > roughly 60% > > of the "issues" so I may be missing some error types, but you should get > > the gist. > > This is all configurable, so we can switch off what we don't agree with. > > AFAIK the > > tool supports custom rules also so we could add our own (locally or > > upstream) for > > commonly seen issues. > > > > Thanks for sharing. > > It'd be nice to strip this down to errors that will actually negatively > affect the docs getting rendered properly. If it's something that breaks > viewing the doc on github or breaks the HTML version in dist-docs, that's > something that would be nice to catch automatically. I'm personally much > less concerned about the style nits. Yeah, I agree. There's some stuff in there that definitely doesn't need to be there. So the idea is good, but what about the actual tool? Are we happy to proceed with this 'mdl' tool or should we use something else (or just build one, a la OpenStack :)) If the former, I should note that the package is not available on any distro yet: is linking to GitHub suitable? Stephen
On Fri, Nov 13, 2015 at 6:03 PM, Finucane, Stephen < stephen.finucane@intel.com> wrote: > On 13 Nov 13:18, Russell Bryant wrote: > > On Fri, Nov 13, 2015 at 11:40 AM, Finucane, Stephen < > > stephen.finucane@intel.com> wrote: > > > > > On 13 Nov 14:08, Russell Bryant wrote: > > > > On Thu, Nov 12, 2015 at 9:18 PM, Stephen Finucane < > > > stephen.finucane@intel.com> > > > > wrote: > > > > > > > > This provides a quick, easy way to use the 'mdl' executable > provided > > > > in markdowlinter to validate documentation. > > > > > > > > This change does not resolve any of the issues this linter > raises - > > > > these will need to be done in a follow up patch. > > > > > > > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > > > > --- > > > > INSTALL.md | 5 +++++ > > > > Makefile.am | 4 ++++ > > > > 2 files changed, 9 insertions(+) > > > > > > > > diff --git a/INSTALL.md b/INSTALL.md > > > > index 906825a..7311915 100644 > > > > --- a/INSTALL.md > > > > +++ b/INSTALL.md > > > > @@ -111,6 +111,11 @@ To run the unit tests, you also need: > > > > - Perl. Version 5.10.1 is known to work. Earlier versions > should > > > > also work. > > > > > > > > +If you are going to modify Open vSwitch documentation, please > > > consider > > > > +installing the following to validate your changes: > > > > + > > > > + - "markdownlint" (https://github.com/mivok/markdownlint) > > > > + > > > > The ovs-vswitchd.conf.db(5) manpage will include an E-R > diagram, in > > > > formats other than plain text, only if you have the following: > > > > > > > > diff --git a/Makefile.am b/Makefile.am > > > > index 966ba27..d58cb59 100644 > > > > --- a/Makefile.am > > > > +++ b/Makefile.am > > > > @@ -377,6 +377,10 @@ dist-docs: > > > > VERSION=$(VERSION) $(srcdir)/build-aux/dist-docs > $(srcdir) > > > $(docs) > > > > .PHONY: dist-docs > > > > > > > > +lint-docs: > > > > + mdl $(docs) > > > > +.PHONY: dist-docs > > > > > > > > > > > > You have a typo here: s/dist-docs/lint-docs/ > > > > > > This is what happens when I write code at night :) > > > > > > > What results does this come up with for the current docs? That'd > > > probably help > > > > demonstrate the value. I got errors trying to install mdl and > decided > > > to just > > > > ask for now instead. :-) > > > > > > > > In general though, I think this seems like a fine idea. > > > > > > Are you ready for this (starts humming the Space Jam tune)? I've > dropped > > > roughly 60% > > > of the "issues" so I may be missing some error types, but you should > get > > > the gist. > > > This is all configurable, so we can switch off what we don't agree > with. > > > AFAIK the > > > tool supports custom rules also so we could add our own (locally or > > > upstream) for > > > commonly seen issues. > > > > > > > Thanks for sharing. > > > > It'd be nice to strip this down to errors that will actually negatively > > affect the docs getting rendered properly. If it's something that breaks > > viewing the doc on github or breaks the HTML version in dist-docs, that's > > something that would be nice to catch automatically. I'm personally > much > > less concerned about the style nits. > > Yeah, I agree. There's some stuff in there that definitely doesn't need to > be > there. > > So the idea is good, but what about the actual tool? Are we happy to > proceed > with this 'mdl' tool or should we use something else (or just build one, a > la > OpenStack :)) If the former, I should note that the package is not > available > on any distro yet: is linking to GitHub suitable? > I'm not that concerned as long as running the tool is optional like you have it in the RFC. It's a bummer that it's not packaged and it wouldn't install cleanly for me on Fedora (though I honestly only tried for about 2 minutes). I haven't looked at alternatives and I certainly don't think we should write our own. I'll take your word for it if you've looked around at what's out there and this looks best. :-) I'll try it again later ... I'm curious what real errors this catches that we wouldn't already catch with just converting it to HTML.
On 15 Nov 19:10, Russell Bryant wrote: [snip] > > > Thanks for sharing. > > > > > > It'd be nice to strip this down to errors that will actually negatively > > > affect the docs getting rendered properly. If it's something that breaks > > > viewing the doc on github or breaks the HTML version in dist-docs, that's > > > something that would be nice to catch automatically. I'm personally > > much > > > less concerned about the style nits. > > > > Yeah, I agree. There's some stuff in there that definitely doesn't need to > > be > > there. > > > > So the idea is good, but what about the actual tool? Are we happy to > > proceed > > with this 'mdl' tool or should we use something else (or just build one, a > > la > > OpenStack :)) If the former, I should note that the package is not > > available > > on any distro yet: is linking to GitHub suitable? > > > > I'm not that concerned as long as running the tool is optional like you > have it in the RFC. It's a bummer that it's not packaged and it wouldn't > install cleanly for me on Fedora (though I honestly only tried for about 2 > minutes). I haven't looked at alternatives and I certainly don't think we > should write our own. I'll take your word for it if you've looked around > at what's out there and this looks best. :-) I'll try it again later ... > > I'm curious what real errors this catches that we wouldn't already catch > with just converting it to HTML. > > -- > Russell Bryant I don't think there's such a thing as a "real error" in Markdown - the output is just not as expected [1]. This tool does, however, seem to catch some obvious errors. For example, in the below we see what should be two lines of text and a code block Some text echo "Hello, world" some more text But because the code isn't indented with the correct number of spaces (4+), this isn't detected as code. The DPDK doc [2], which I'm trying to rework at the moment, is littered with these mistakes [3]. I'm thinking this tool will catch error like code blocks not being indented enough or bullet points at funny levels but that's probably as good as we can get. How's that sound? Stephen [1] http://openvswitch.org/pipermail/dev/2015-February/051234.html [2] https://raw.githubusercontent.com/openvswitch/ovs/89108874d59364313f9e5e192ba8ca00a2771d93/INSTALL.DPDK.md [3] https://github.com/openvswitch/ovs/blob/89108874d59364313f9e5e192ba8ca00a2771d93/INSTALL.DPDK.md#dpdk-vhost-cuse-vm-configuration-with-libvirt
On Fri, Nov 20, 2015 at 4:33 PM, Finucane, Stephen < stephen.finucane@intel.com> wrote: > On 15 Nov 19:10, Russell Bryant wrote: > > [snip] > > > > > Thanks for sharing. > > > > > > > > It'd be nice to strip this down to errors that will actually > negatively > > > > affect the docs getting rendered properly. If it's something that > breaks > > > > viewing the doc on github or breaks the HTML version in dist-docs, > that's > > > > something that would be nice to catch automatically. I'm personally > > > much > > > > less concerned about the style nits. > > > > > > Yeah, I agree. There's some stuff in there that definitely doesn't > need to > > > be > > > there. > > > > > > So the idea is good, but what about the actual tool? Are we happy to > > > proceed > > > with this 'mdl' tool or should we use something else (or just build > one, a > > > la > > > OpenStack :)) If the former, I should note that the package is not > > > available > > > on any distro yet: is linking to GitHub suitable? > > > > > > > I'm not that concerned as long as running the tool is optional like you > > have it in the RFC. It's a bummer that it's not packaged and it wouldn't > > install cleanly for me on Fedora (though I honestly only tried for about > 2 > > minutes). I haven't looked at alternatives and I certainly don't think > we > > should write our own. I'll take your word for it if you've looked around > > at what's out there and this looks best. :-) I'll try it again later > ... > > > > I'm curious what real errors this catches that we wouldn't already catch > > with just converting it to HTML. > > > > -- > > Russell Bryant > > I don't think there's such a thing as a "real error" in Markdown - the > output > is just not as expected [1]. This tool does, however, seem to catch some > obvious errors. For example, in the below we see what should be two lines > of > text and a code block > > Some text > > echo "Hello, world" > > some more text > > But because the code isn't indented with the correct number of spaces (4+), > this isn't detected as code. The DPDK doc [2], which I'm trying to rework > at > the moment, is littered with these mistakes [3]. I'm thinking this tool > will > catch error like code blocks not being indented enough or bullet points at > funny levels but that's probably as good as we can get. How's that sound? > Sounds good to me.
On Fri, Nov 13, 2015 at 02:18:40AM +0000, Stephen Finucane wrote: > This provides a quick, easy way to use the 'mdl' executable provided > in markdowlinter to validate documentation. > > This change does not resolve any of the issues this linter raises - > these will need to be done in a follow up patch. > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> I like checkers, but I like them even more when they can run automatically every time I do a build, and fail the build if they find a problem. It's easy enough to arrange for that by only running "mdl" if it's installed, but can "mdl" be made to complain only about problems that we consider in OVS to be important ones? That might have to be figured out by experiment, of course.
On 24 Nov 22:05, Ben Pfaff wrote: > On Fri, Nov 13, 2015 at 02:18:40AM +0000, Stephen Finucane wrote: > > This provides a quick, easy way to use the 'mdl' executable provided > > in markdowlinter to validate documentation. > > > > This change does not resolve any of the issues this linter raises - > > these will need to be done in a follow up patch. > > > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > > I like checkers, but I like them even more when they can run > automatically every time I do a build, and fail the build if they find a > problem. It's easy enough to arrange for that by only running "mdl" if > it's installed, but can "mdl" be made to complain only about problems > that we consider in OVS to be important ones? That might have to be > figured out by experiment, of course. Yeah, we can configure the checkers that we want to run and likely add some of our own down the line. I'm doing this at the moment and the non-RFC patch will include this configuration. Stephen
diff --git a/INSTALL.md b/INSTALL.md index 906825a..7311915 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -111,6 +111,11 @@ To run the unit tests, you also need: - Perl. Version 5.10.1 is known to work. Earlier versions should also work. +If you are going to modify Open vSwitch documentation, please consider +installing the following to validate your changes: + + - "markdownlint" (https://github.com/mivok/markdownlint) + The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in formats other than plain text, only if you have the following: diff --git a/Makefile.am b/Makefile.am index 966ba27..d58cb59 100644 --- a/Makefile.am +++ b/Makefile.am @@ -377,6 +377,10 @@ dist-docs: VERSION=$(VERSION) $(srcdir)/build-aux/dist-docs $(srcdir) $(docs) .PHONY: dist-docs +lint-docs: + mdl $(docs) +.PHONY: dist-docs + include Documentation/automake.mk include m4/automake.mk include lib/automake.mk
This provides a quick, easy way to use the 'mdl' executable provided in markdowlinter to validate documentation. This change does not resolve any of the issues this linter raises - these will need to be done in a follow up patch. Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> --- INSTALL.md | 5 +++++ Makefile.am | 4 ++++ 2 files changed, 9 insertions(+)