Opened 3 years ago

Closed 3 years ago

#5380 defect closed fixed (fixed)

website-template.tpl is out of date

Reported by: thijs Owned by: thijs
Priority: normal Milestone: totally automated release infrastructure
Component: core Keywords: documentation
Cc: thijs, jesstess Branch: branches/website-template-5380
(diff, github, buildbot, log)
Author: thijs Launchpad Bug:

Description (last modified by thijs)

(from #4693) The release document looks wrong, since it does direct the release manager to use trunk/doc/core/howto/template.tpl to generate the website documentation. Instead, it should be directing the release manager to use trunk/doc/core/howto/website-template.tpl.

In turn, it seems website-template.tpl has gotten somewhat out of date for a few reasons:

  • It is missing the version footer.
  • It is missing at sitemeter and google analytics (adding this would remove a couple gross manual steps from the release process).
  • It should have a link to the latest version of the docs, as was just recently added to the api docs pages.

Attachments (1)

website-template.tpl (2.4 KB) - added by thijs 3 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 3 years ago by thijs

  • Author set to thijs
  • Branch set to branches/website-template-5380

(In [33111]) Branching to 'website-template-5380'

comment:2 Changed 3 years ago by thijs

(In [33112]) Reference the website-template.tpl instead, refs #5380

comment:3 Changed 3 years ago by thijs

(In [33113]) add version link and site javascript, refs #5380

comment:4 Changed 3 years ago by thijs

(In [33114]) correct index link, add current link. refs #5380

comment:5 in reply to: ↑ description Changed 3 years ago by thijs

  • Keywords review added

Replying to thijs:

(from #4693) The release document looks wrong, since it does direct the release manager to use trunk/doc/core/howto/template.tpl to generate the website documentation. Instead, it should be directing the release manager to use trunk/doc/core/howto/website-template.tpl.

This has been fixed on ReleaseProcess. Also added a link to this ticket.

In turn, it seems template.tpl has gotten somewhat out of date for a few reasons:

This should be website-template.tpl.

  • It is missing the version footer.

Added.

  • It is missing at sitemeter and google analytics (adding this would remove a couple gross manual steps from the release process).

Added. Still needs the find/sed fixes as described on ReleaseProcess.

  • It should have a link to the latest version of the docs, as was just recently added to the api docs pages.

Added, but this also introduces a new find/sed line that needs to be run on the docs, I'll add it here for reference (needs to be added to the ReleaseProcess document when this ticket is merged to trunk).

find /tmp/twisted-release/doc -name '*.html' | xargs sed -i 's/container.style.display = ""/container.style.display = ""/'

Up for review.

comment:6 Changed 3 years ago by jesstess

  • Owner set to jesstess

comment:7 Changed 3 years ago by jesstess

  • Cc jesstess added
  • Keywords review removed
  • Owner changed from jesstess to thijs

Thanks for working on this, thijs! A few comments:

Can we use the newer versions of the Javascript for these tracking sites and update ReleaseProcess accordingly?

comment:8 Changed 3 years ago by thijs

  • Status changed from new to assigned

comment:9 Changed 3 years ago by thijs

(In [33150]) Use latest google analytics and sitemeter scripts, refs #5380

comment:10 Changed 3 years ago by thijs

  • Keywords review added
  • Owner thijs deleted
  • Status changed from assigned to new

Thanks for the review jesstess. I updated the scripts and also got rid of that additional find command from comment 5 by using single qoutes instead of double, this seems to work around #4544 (so once this lands we can get rid of those analytics instructions on ReleaseProcess and move #4544 and #4545 out of the regular-releases milestone. Would be good to have this in twisted 11.1 or the release manager will run into problems with the updated ReleaseProcess instructions.

comment:11 Changed 3 years ago by thijs

(In [33151]) Don't wait for sitemeter, refs #5380

comment:12 Changed 3 years ago by thijs

Marked #5397 as duplicate of this ticket.

comment:13 Changed 3 years ago by jesstess

  • Keywords review removed
  • Owner set to thijs

Sorry that it didn't make it into the release, but this looks great and ready to merge.

comment:14 Changed 3 years ago by thijs

  • Resolution set to fixed
  • Status changed from new to closed

(In [33195]) Merge website-template-5380: Including version footer, a link to the latest version of the documentation,
and the latest analytics scripts for google and sitemeter in the website template for the howto documentation.

Author: thijs
Reviewer: jesstess
Fixes: #5380

comment:15 Changed 3 years ago by exarkun

  • Resolution fixed deleted
  • Status changed from closed to reopened

(In [33201]) Revert r33195 - test suite regression

The release automation tests in twisted.python.test.test_release fail with this
change due to the new name of the required template file.

Reopens: #5380

comment:16 Changed 3 years ago by thijs

  • Description modified (diff)
  • Status changed from reopened to new

test_release tests against template.tpl, but actual Twisted releases should use the updated website-template.tpl that includes the current link and analytics code.

exarkun mentioned that website-template belongs in website repository (lp:twisted-trac?) instead of the Twisted repository.

bin/admin/build-tarballs is hard-coded to use template.tpl, but this branch changed it to website-template.tpl, causing the test suite to fail.

So I suggest to add a template argument to the bin/admin/build-tarballs, if the argument is missing the default template.tpl will be used, included in the twisted source tree.

ReleaseProcess should be updated and explain how to obtain and use the website-template.tpl from the website repository.

comment:17 Changed 3 years ago by thijs

(In [33242]) Add 3rd parameter to build-tarballs, specifying the path to the custom (website) template. refs #5380

Changed 3 years ago by thijs

comment:18 Changed 3 years ago by thijs

  • Keywords review added
  • Owner changed from thijs to exarkun

website-template.tpl has been deleted from the source tree and attached to this ticket. Assigning it to exarkun so he can commit that file in the twisted website repository (I don't have commit rights there).

Release managers should now use this command to build the tarballs:

$ ./bin/admin/build-tarballs . /tmp/twisted-release /path/to/website-template.tpl

Without specifying a template the default trunk/doc/core/howto/template.tpl will be used.

comment:19 follow-ups: Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to thijs

The website is lp:twisted-website. Anyone can make a bzr branch and push it to launchpad, and I think you also have permission to push to the development focus branch.

  1. The templatePath API documentation is missing a word or something - Path to the template file that used for the howto documentation.
  2. In buildAllTarballs, the new isinstance check seems weird. A direct check against None would be better. The entire check might be better if done in DistributionBuilder._buildDocInDir or DistributionBuilder.build. Then again, since there is no code to actually handle TemplateDoesNotExist, there doesn't seem to be much reason to make the check at all. An unhandled OSError explaining that a file does not exist is about as user-friendly as an unhandled TemplateDoesNotExist saying the same thing.
  3. The new optional template path command line argument isn't very discoverable. The usage errors that the script will emit does not mention it. This command may be complex enough to warrant the use of an Options subclass now. At the very least, the usage information given in the error case of BuildTarballsScript.main needs to mention the new option.

Thanks.

comment:20 in reply to: ↑ 19 Changed 3 years ago by thijs

  • Status changed from new to assigned

Replying to exarkun:

The website is lp:twisted-website. Anyone can make a bzr branch and push it to launchpad, and I think you also have permission to push to the development focus branch.

The template file has been added to the docs folder, I'll update the wiki accordingly.

comment:21 Changed 3 years ago by thijs

(In [33504]) Address review comments, refs #5380

comment:22 in reply to: ↑ 19 Changed 3 years ago by thijs

  • Keywords review added
  • Owner thijs deleted
  • Status changed from assigned to new

Replying to exarkun:

  1. The templatePath API documentation is missing a word or something - Path to the template file that used for the howto documentation.

Fixed.

  1. In buildAllTarballs, the new isinstance check seems weird. A direct check against None would be better. The entire check might be better if done in DistributionBuilder._buildDocInDir or DistributionBuilder.build. Then again, since there is no code to actually handle TemplateDoesNotExist, there doesn't seem to be much reason to make the check at all. An unhandled OSError explaining that a file does not exist is about as user-friendly as an unhandled TemplateDoesNotExist saying the same thing.

Reverted it to OSError, eg.:

build-docs.py: /path/to/website-template.tpl: No such file or directory
  1. The new optional template path command line argument isn't very discoverable. The usage errors that the script will emit does not mention it. This command may be complex enough to warrant the use of an Options subclass now. At the very least, the usage information given in the error case of BuildTarballsScript.main needs to mention the new option.

Added a description for the third optional argument.

comment:23 follow-up: Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to thijs

Thanks!

  1. Maybe the repeated child calls in _release.py, around line 985, could be replaced with use of the descendant method.
  2. The "re" module is imported in release.py but not used, maybe that could be removed.
  3. BuildTarballsScript.main is missing some test coverage. The len(args) == 2 case appears not to be exercised now.

I am now a little bit confused. I tried to think about when someone would want to pass an alternate template path to build-tarballs but I couldn't figure anything out. The facility certainly doesn't hurt, but it is superfluous with the current release process, right? It's the yet nonexistent build-docs script that will really need this, I think?

Otherwise looks great. I started a build. If that looks good, and after addressing the above points, please merge. Thanks.

comment:24 Changed 3 years ago by thijs

(In [33528]) address review comments, refs #5380

comment:25 in reply to: ↑ 23 Changed 3 years ago by thijs

  • Status changed from new to assigned

Replying to exarkun:

Thanks!

  1. Maybe the repeated child calls in _release.py, around line 985, could be replaced with use of the descendant method.
  2. The "re" module is imported in release.py but not used, maybe that could be removed.
  3. BuildTarballsScript.main is missing some test coverage. The len(args) == 2 case appears not to be exercised now.

Fixed.

I am now a little bit confused. I tried to think about when someone would want to pass an alternate template path to build-tarballs but I couldn't figure anything out. The facility certainly doesn't hurt, but it is superfluous with the current release process, right? It's the yet nonexistent build-docs script that will really need this, I think?

Yeah, this script is the temporary build-docs script. Once #4500 lands we will have to rewrite this template stuff anyway, but nobody knows when that lore2sphinx is going to complete, so instead of adding this 'lore theme' to the trunk it now lives in lp:twisted-website as you suggested, where the sphinx theme will live as well I imagine.

comment:26 Changed 3 years ago by thijs

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [33529]) Merge website-template-5380: Moved outdated website template to the lp:twisted-website repository, used for building the howto documentation.

Author: thijs
Reviewer: jesstess, exarkun
Fixes: #5380

Note: See TracTickets for help on using tickets.