[libvirt PATCH] docs: add 'edit this page' link to footer of every page

Daniel P. Berrangé posted 1 patch 4 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200408152141.1182150-1-berrange@redhat.com
docs/Makefile.am | 5 +++++
docs/page.xsl    | 7 +++++++
docs/site.xsl    | 1 +
docs/subsite.xsl | 1 +
4 files changed, 14 insertions(+)
[libvirt PATCH] docs: add 'edit this page' link to footer of every page
Posted by Daniel P. Berrangé 4 years ago
To encourage contributors to make changes to the main website, add a
footer link to every page which links to the corresponding source file
in git. With gitlab, they are able to edit content directly in the web
browser and then submit a merge request. This gives a way to contribute
content that is arguably easier than our wiki which requires manual
account creation, while this will also benefit from maintainer review.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/Makefile.am | 5 +++++
 docs/page.xsl    | 7 +++++++
 docs/site.xsl    | 1 +
 docs/subsite.xsl | 1 +
 4 files changed, 14 insertions(+)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 61862c4ab7..483414105d 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -429,6 +429,10 @@ manpages/%.html.in: manpages/%.rst
 %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
 		$(acl_generated)
 	$(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
+          genhtmlin=`echo $@ | sed -e 's/.tmp/.in/'`; \
+          rst=`echo $@ | sed -e 's/.html.tmp/.rst/'`; \
+          src="$$genhtmlin"; \
+          test -f "$$genhtmlin" && src="$$rst"; \
 	  dir=`dirname $@` ; \
 	  if test "$$dir" = "."; \
 	  then \
@@ -438,6 +442,7 @@ manpages/%.html.in: manpages/%.rst
 	    style=subsite.xsl; \
 	  fi; \
 	  $(XSLTPROC) --stringparam pagename $$name \
+            --stringparam pagesrc $$src \
 	    --stringparam builddir '$(abs_top_builddir)' \
 	    --stringparam timestamp $(timestamp) --nonet \
 	    $(top_srcdir)/docs/$$style $< > $@ \
diff --git a/docs/page.xsl b/docs/page.xsl
index ddae5ab508..6e3ed6c688 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -77,6 +77,7 @@
   <!-- This is the master page structure -->
   <xsl:template match="/" mode="page">
     <xsl:param name="pagename"/>
+    <xsl:param name="pagesrc"/>
     <xsl:param name="timestamp"/>
     <xsl:text disable-output-escaping="yes">&lt;!DOCTYPE html&gt;
 </xsl:text>
@@ -150,6 +151,12 @@
           </div>
         </div>
         <div id="footer">
+          <div id="contact">
+            <h3>Contribute</h3>
+            <ul>
+              <li><a href="https://gitlab.com/libvirt/libvirt/-/blob/master/docs/{$pagesrc}">edit this page</a></li>
+            </ul>
+          </div>
           <div id="contact">
             <h3>Contact</h3>
             <ul>
diff --git a/docs/site.xsl b/docs/site.xsl
index c9d41f2c93..73ddd8ca6b 100644
--- a/docs/site.xsl
+++ b/docs/site.xsl
@@ -27,6 +27,7 @@
   <xsl:template match="/">
     <xsl:apply-templates select="." mode="page">
       <xsl:with-param name="pagename" select="$pagename"/>
+      <xsl:with-param name="pagesrc" select="$pagesrc"/>
       <xsl:with-param name="timestamp" select="$timestamp"/>
     </xsl:apply-templates>
   </xsl:template>
diff --git a/docs/subsite.xsl b/docs/subsite.xsl
index f56ac0491c..bfd34eebd2 100644
--- a/docs/subsite.xsl
+++ b/docs/subsite.xsl
@@ -17,6 +17,7 @@
   <xsl:template match="/">
     <xsl:apply-templates select="." mode="page">
       <xsl:with-param name="pagename" select="$pagename"/>
+      <xsl:with-param name="pagesrc" select="$pagesrc"/>
       <xsl:with-param name="timestamp" select="$timestamp"/>
     </xsl:apply-templates>
   </xsl:template>
-- 
2.24.1

Re: [libvirt PATCH] docs: add 'edit this page' link to footer of every page
Posted by Ján Tomko 4 years ago
On a Wednesday in 2020, Daniel P. Berrangé wrote:
>To encourage contributors to make changes to the main website, add a
>footer link to every page which links to the corresponding source file
>in git. With gitlab, they are able to edit content directly in the web
>browser and then submit a merge request.

Should this patch wait until we switch to merge requests for libvirt?

(Also, for repositories where I can push directly, the text about
opening a merge request is not present and pressing 'submit changes'
pushes a commit to master right away - can notifications be set for
push events too? I don't seem to be getting any, despite setting
'watch' for the libvirt project)

>This gives a way to contribute
>content that is arguably easier than our wiki which requires manual
>account creation, while this will also benefit from maintainer review.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> docs/Makefile.am | 5 +++++
> docs/page.xsl    | 7 +++++++
> docs/site.xsl    | 1 +
> docs/subsite.xsl | 1 +
> 4 files changed, 14 insertions(+)
>
>@@ -150,6 +151,12 @@
>           </div>
>         </div>
>         <div id="footer">
>+          <div id="contact">
>+            <h3>Contribute</h3>
>+            <ul>
>+              <li><a href="https://gitlab.com/libvirt/libvirt/-/blob/master/docs/{$pagesrc}">edit this page</a></li>

Consider s/blob/edit/ to go directly to the editing page, at the cost of
showing the gitlab login page instead of the source file to users who
aren't logged in.

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH] docs: add 'edit this page' link to footer of every page
Posted by Daniel P. Berrangé 4 years ago
On Wed, Apr 08, 2020 at 06:08:59PM +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Daniel P. Berrangé wrote:
> > To encourage contributors to make changes to the main website, add a
> > footer link to every page which links to the corresponding source file
> > in git. With gitlab, they are able to edit content directly in the web
> > browser and then submit a merge request.
> 
> Should this patch wait until we switch to merge requests for libvirt?
> 
> (Also, for repositories where I can push directly, the text about
> opening a merge request is not present and pressing 'submit changes'
> pushes a commit to master right away - can notifications be set for
> push events too? I don't seem to be getting any, despite setting
> 'watch' for the libvirt project)

If you click the "watch" icon against a proiject and select "custom"
you can see a list of the event types that it can send on, and there's
no mention of "direct pushes". So I don't believe this is possible.

With current setup those with direct commit privs on libvirt
can directly commit from the web UI. We must trust them not to
abuse that, as we trust them not to abuse their CLI based push
privileges.

3rd parties without commit privs will be prompted to fork the
repository when they press the "Edit" button, before they can
make changes. After that they'll need to submit the changes
back to libvirt, in accordance with our stated contribution
policy.

So although my commit message here talks about merge requests,
for now we'd still expect email submission. They should just
follow whatever we state in CONTRIBUTING.md if they're playing
nicely.

Once we do switch to a merge request flow, the branch protection
rules can be set to forbid direct pushes, and that means even
libvirt maintainers will have to go for the fork route in the
same way as 3rd parties do today.


> > This gives a way to contribute
> > content that is arguably easier than our wiki which requires manual
> > account creation, while this will also benefit from maintainer review.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > docs/Makefile.am | 5 +++++
> > docs/page.xsl    | 7 +++++++
> > docs/site.xsl    | 1 +
> > docs/subsite.xsl | 1 +
> > 4 files changed, 14 insertions(+)
> > 
> > @@ -150,6 +151,12 @@
> >           </div>
> >         </div>
> >         <div id="footer">
> > +          <div id="contact">
> > +            <h3>Contribute</h3>
> > +            <ul>
> > +              <li><a href="https://gitlab.com/libvirt/libvirt/-/blob/master/docs/{$pagesrc}">edit this page</a></li>
> 
> Consider s/blob/edit/ to go directly to the editing page, at the cost of
> showing the gitlab login page instead of the source file to users who
> aren't logged in.

That URL change only makes a difference for the few of us who have
direct commit privileges to libvirt. For anyone else, if they
follow the /edit/ link, they'll get redirected to the /blob/
linnk again, and prompted to fork the repo. Given that /edit/
has the downside of showing the login screen, and the main
target audience is 3rd party people, not main libvirt maintrainers,
I think we can just stick with /blob/

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH] docs: add 'edit this page' link to footer of every page
Posted by Ján Tomko 4 years ago
On a Wednesday in 2020, Daniel P. Berrangé wrote:
>On Wed, Apr 08, 2020 at 06:08:59PM +0200, Ján Tomko wrote:
>> On a Wednesday in 2020, Daniel P. Berrangé wrote:
>> > This gives a way to contribute
>> > content that is arguably easier than our wiki which requires manual
>> > account creation, while this will also benefit from maintainer review.
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> > docs/Makefile.am | 5 +++++
>> > docs/page.xsl    | 7 +++++++
>> > docs/site.xsl    | 1 +
>> > docs/subsite.xsl | 1 +
>> > 4 files changed, 14 insertions(+)
>> >
>> > @@ -150,6 +151,12 @@
>> >           </div>
>> >         </div>
>> >         <div id="footer">
>> > +          <div id="contact">
>> > +            <h3>Contribute</h3>
>> > +            <ul>
>> > +              <li><a href="https://gitlab.com/libvirt/libvirt/-/blob/master/docs/{$pagesrc}">edit this page</a></li>
>>
>> Consider s/blob/edit/ to go directly to the editing page, at the cost of
>> showing the gitlab login page instead of the source file to users who
>> aren't logged in.
>
>That URL change only makes a difference for the few of us who have
>direct commit privileges to libvirt. For anyone else, if they
>follow the /edit/ link, they'll get redirected to the /blob/
>linnk again, and prompted to fork the repo.

Ah, right. I tried it on your fork of a repo that I also forked,
so the message about forking did not show up for me.

Jano

>Given that /edit/
>has the downside of showing the login screen, and the main
>target audience is 3rd party people, not main libvirt maintrainers,
>I think we can just stick with /blob/
>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
Re: [libvirt PATCH] docs: add 'edit this page' link to footer of every page
Posted by Andrea Bolognani 4 years ago
On Wed, 2020-04-08 at 16:21 +0100, Daniel P. Berrangé wrote:
> +++ b/docs/Makefile.am
> @@ -429,6 +429,10 @@ manpages/%.html.in: manpages/%.rst
>  %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
>  		$(acl_generated)
>  	$(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
> +          genhtmlin=`echo $@ | sed -e 's/.tmp/.in/'`; \
> +          rst=`echo $@ | sed -e 's/.html.tmp/.rst/'`; \
> +          src="$$genhtmlin"; \
> +          test -f "$$genhtmlin" && src="$$rst"; \

The indentation is all wrong here.

> @@ -438,6 +442,7 @@ manpages/%.html.in: manpages/%.rst
>  	    style=subsite.xsl; \
>  	  fi; \
>  	  $(XSLTPROC) --stringparam pagename $$name \
> +            --stringparam pagesrc $$src \

Here too.

> +++ b/docs/page.xsl
> @@ -150,6 +151,12 @@
>            </div>
>          </div>
>          <div id="footer">
> +          <div id="contact">

The id is wrong, you need a new one for this footer section.

> +            <h3>Contribute</h3>
> +            <ul>
> +              <li><a href="https://gitlab.com/libvirt/libvirt/-/blob/master/docs/{$pagesrc}">edit this page</a></li>
> +            </ul>
> +          </div>

I think the "edit this page" link would actually be more prominent
if it were the third one (last one on the left) in the footer rather
than the first one. Please give it a try.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH] docs: add 'edit this page' link to footer of every page
Posted by Daniel P. Berrangé 4 years ago
On Wed, Apr 08, 2020 at 07:05:17PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-08 at 16:21 +0100, Daniel P. Berrangé wrote:
> > +++ b/docs/Makefile.am
> > @@ -429,6 +429,10 @@ manpages/%.html.in: manpages/%.rst
> >  %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
> >  		$(acl_generated)
> >  	$(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
> > +          genhtmlin=`echo $@ | sed -e 's/.tmp/.in/'`; \
> > +          rst=`echo $@ | sed -e 's/.html.tmp/.rst/'`; \
> > +          src="$$genhtmlin"; \
> > +          test -f "$$genhtmlin" && src="$$rst"; \
> 
> The indentation is all wrong here.

Opps yes. I will love it when we remove make :-)

> 
> > @@ -438,6 +442,7 @@ manpages/%.html.in: manpages/%.rst
> >  	    style=subsite.xsl; \
> >  	  fi; \
> >  	  $(XSLTPROC) --stringparam pagename $$name \
> > +            --stringparam pagesrc $$src \
> 
> Here too.
> 
> > +++ b/docs/page.xsl
> > @@ -150,6 +151,12 @@
> >            </div>
> >          </div>
> >          <div id="footer">
> > +          <div id="contact">
> 
> The id is wrong, you need a new one for this footer section.

Yes

> 
> > +            <h3>Contribute</h3>
> > +            <ul>
> > +              <li><a href="https://gitlab.com/libvirt/libvirt/-/blob/master/docs/{$pagesrc}">edit this page</a></li>
> > +            </ul>
> > +          </div>
> 
> I think the "edit this page" link would actually be more prominent
> if it were the third one (last one on the left) in the footer rather
> than the first one. Please give it a try.

Sure, that works.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|