Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13290 closed (fixed)

dependency on external jar file for admin js minification needs better documentation

Reported by: gabrielhurley Owned by: gabrielhurley
Component: Documentation Version: 1.1
Severity: Keywords: admin js compress
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In revision [12371] jezdez added contrib/admin/media/js/compress.py, which is very handy but is also dependent on the developer having downloaded and installed Google's Closure Compiler. That's further complicated by the fact that Closure Compiler relies on Java 6 (or Java 1.6 on Mac OS X, which was not the default shipped version of Java until Snow Leopard).

While I eventually figured out what compiler it was looking for from the help text on the -c flag ( "path to closure compiler jar file"), having proper documentation of the external requirement, where to download the compiler from, a note about the potential trouble with Mac OS X, etc. would be more developer-friendly. Comments in the source and/or improved help text are probably the place to add the notes.

I'll write up a patch a bit later.

Attachments (1)

13290_js_compression_docs.diff (3.2 KB) - added by gabrielhurley 4 years ago.
adds info on minifying admin javascript and compress.py

Download all attachments as: .zip

Change History (6)

comment:1 Changed 4 years ago by russellm

  • Component changed from Contrib apps to Documentation
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This needs to be documented in two ways:

  1. That the dependency exists if you want to contribute a fix to Django's admin javascript code
  2. The change in procedure for how to contribute a fix to javascript.

I suspect both of these should be additions to the contributing docs, similar to the way that the process for submitting translation updates is documented.

comment:2 Changed 4 years ago by jezdez

You got me, this needs documentation in the contribution docs. But FWIW, I don't think minification (Russ' bullet point 1) should be a required step for contributed admin JavaScript fixes. We always compile translation files before committing them, too.

Changed 4 years ago by gabrielhurley

adds info on minifying admin javascript and compress.py

comment:3 Changed 4 years ago by gabrielhurley

  • Has patch set
  • Status changed from new to assigned

I've attached a patch as per Russ's earlier direction. It was written before I saw jezdez's comment.

For what it's worth, IMHO it's better to:

  1. "require" people to include the minified version in their patch, knowing that they're human and it'll often be necessary to minify them again before committing, rather than
  2. not require people to include it and have them almost never submit a minified version in the patch.

Also, another question as long as we're talking about it: should the other admin javascript files eventually be transitioned to using compressed versions as well? Currently compress.py only hits 3 files even though it says it compresses "all" of them.

comment:4 Changed 4 years ago by jezdez

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

(In [12969]) Fixed #13290 - Added a section about minification of admin JavaScript files to the contributing docs. Thanks to Gabriel Hurley for the report and patch.

comment:5 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.