282728 - Add a switch to checksetup to skip compiling the templates

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, enhancement)

Description

[Max Kanat-Alexander]

Whenever I'm doing development on checksetup, I have to wait a few minutes for
each test run because of the template precompile.

I'd really like to be able to have a --fast switch or something that would skip
that part. Or maybe a --onlydb switch, that would also skip the module checking.
But that might be dangerous, so perhaps we'd better just stick with the --fast
thing.

Comment 1

[Max Kanat-Alexander]

Attachment: Add --full and --fast

OK, this slightly changes the behavior from bug 177850. Instead of always
deleting the data/template directory, we do so only if --full is specified.

If --fast is specified, we completely skip template compilation.

Comment 2

[Shane H. W. Travis]

+$switch{'fast'} = !$switch{'fast'} && grep(/^--fast$/, @ARGV);

Shouldn't it be !$switch{'full'} in that operation?

Second comment is that I, personally, don't like the fact that you MUST 
indicate either --full or --fast. I strongly believe that --full should be 
implied/default, so that if people continue to run './checksetup.pl' they will 
condinue to get the historically expected behaviour. Giving people the option 
to override it by adding --fast is fine, and certainly doesn't hurt anything... 
but forcing people to explicitly state another parameter just to get the 
old/expected (and documented) behaviour is not user-friendly.

Comment 3

[Max Kanat-Alexander]

(In reply to comment #2)
> but forcing people to explicitly state another parameter just to get the 
> old/expected (and documented) behaviour is not user-friendly.

  It depends who you are. Actually, to me, this is the expected behavior,
because this is how it worked in 2.16. It changed in 2.18.

  But sure, I could agree and say that we could keep doing what we're doing. It
would prevent more support requests, but checksetup would run unnecessarily
slower for the majority of users.

Comment 4

[Shane H. W. Travis]

okay, #1: I mis-read something, and thought that a user would be forced to 
choose either --full or --fast, and that checksetup would not run unless you 
did. This is obviously not the case, so I retract that objection.

#2, however, is that this bug is stealth-changing the behaviour of checksetup 
under the guise of doing nothing more than adding another flag. If you want to 
add the flag but leave it as 2.18 behaviour as default, fine... but if you want 
to add the flag AND revert the behaviour to 2.16 behaviour, then that needs to 
be more clearly delineated in the summary and description so that people know 
what they're agreeing to/arguing about.

I would also like to point out that 'the majority of users' don't run 
checksetup.pl *nearly* so often as the people on the cc: list of this bug. For 
them, my feeling is that checksetup should do a complete job as its default 
behaviour. If options exist to speed things up for developers, that's fine... 
but developers needs shouldn't trump users needs.

Comment 5

[Max Kanat-Alexander]

OK. Yeah, I agree with travis. I'll make the patch less controversial.

Comment 6

[Max Kanat-Alexander]

Attachment: v2 (--no-recompile)

OK, now there's just a --no-recompile switch, and I didn't change the default
behavior.

Comment 7

[Max Kanat-Alexander]

Attachment: v3 (--no-templates and -t)

Comment 8

[Max Kanat-Alexander]

Attachment: v4

<mkanat> travis: That patch has a tiny error in the help message that can be
fixed on checkin.
<travis> I saw that
<travis> and was going to say the same thing...
<travis> my larger comment, though, is the section just below	  my
@templatepaths = ();
<travis> you have no conditional around that. Intentional, or accidental?
<mkanat> travis: Oh.
<mkanat> travis: I'll move the conditional to cover the whole thing.
<travis> Seems to me you could/should just put one giant conditional around
that whole thing
<travis> heh
<travis> what a fantastic idea!
<travis> IRC reviews sure save space on the bugs, but I'm sure people are
wondering, "Why does he keep putting up new patches when nobody has commented
on the old ones yet?"

Comment 9

[Shane H. W. Travis]

Comment on attachment 174734 [details] [diff] [review]
v4

Alright, this passes everything we discussed in IRC, and doesn't look like it
has the potential to hurt anything. r+ by inspection.

Comment 10

[Max Kanat-Alexander]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.348; previous revision: 1.347
done