Re: [EGIT PATCH 08/31] Add dryRun option to Transport and console push
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Aug 19, 2008, 16:28 UTC
- Message-ID
- <20080819162816.GF20947@spearce.org>
- In-Reply-To
- <1219005852-21496-9-git-send-email-marek.zawirski@gmail.com>
Marek Zawirski <marek.zawirski@gmail.com> 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.
Show 11 quoted lines
> 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.