Subject: Re: [EGIT PATCH 08/31] Add dryRun option to Transport and console push

From: "Shawn O. Pearce" <[email-masked]>
Date: Tue, 19 Aug 2008 09:28:16 -0700

Marek Zawirski <[email-masked]> wrote:
> Implementation of C Git --dry-run behavior for push operation.
> It allows investigating possible push result, while not performing real
> push operation - not updating remote refs.
 
> diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java
> index a952309..f5b24c6 100644
> --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java
> +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java
> @@ -85,6 +85,9 @@ class Push extends TextBuiltin {
>  
>  	@Option(name = "--receive-pack", metaVar = "path")
>  	private String receivePack;
> +	
> +	@Option(name = "--dry-run")
> +	private boolean dryRun = Transport.DEFAULT_DRY_RUN;

Having a boolean constant DEFAULT_DRY_RUN = false is sort of
overkill in my opinion.  Most people would assume a boolean with
no value assigned and not marked final will default to false.
Setting something like dryRun to false by default is reasonable,
as you usually want it to actually execute.

What's worse though is if one day someone changes
Transport.DEFAULT_DRY_RUN = true in the library.  When that happens
the push command line tool will never function as there is no
"--no-dry-run" to set dryRun = false.

So this assignment is likely not a good idea.

-- 
Shawn.