website-template.tpl is out of date

[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.

[thijs]

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

[thijs]

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

[thijs]

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

[thijs]

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

[thijs]

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.

[jesstess]

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

• urchin.js is from 2007, and it looks like there's a newer recommended way of adding Google Analytics tracking:  http://www.google.com/support/googleanalytics/bin/answer.py?answer=174090.
• Site Meter similarly has a slightly updated version of its tracking javascript that doesn't use the WEBBOT tag:  http://support.sitemeter.com/index.php?_m=knowledgebase&_a=viewarticle&kbarticleid=80

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

[thijs]

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

[thijs]

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.

[thijs]

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

[thijs]

Marked #5397 as duplicate of this ticket.

[jesstess]

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

[thijs]

(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

[exarkun]

(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

[thijs]

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.

[thijs]

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

[thijs]

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.

[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.

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.

[thijs]

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.

[thijs]

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

[thijs]

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.

[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.

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.

[thijs]

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

[thijs]

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. 1. The "re" module is imported in release.py but not used, maybe that could be removed. 1. 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.

[thijs]

(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