RFR: Add tool for importing webrev patches
Erik Duveblad via github.com
duke at openjdk.java.net
Wed Sep 4 12:33:31 UTC 2019
On Fri, 5 Jul 2019 10:01:57 GMT, JornVernee via github.com <duke at openjdk.java.net> wrote:
> As discussed on the mailing list: https://mail.openjdk.java.net/pipermail/skara-dev/2019-July/000176.html
>
> This PR adds a tool for importing webrev `.patch` files, either from a webrev url or local file path into a repository.
>
> I've named the tool `wimport` as short for `webrev-import` (I thought the full name sounded a bit too much like a sub-command of `webrev`). This will first checkout a new branch, then apply the patch, and then create a commit for the patch with the message `Imported patch 'NAME'`, similar to what is done by mq. Afterwards a user can just delete the new branch when they're done with e.g. reviewing the webrev. Although, there is also an option to directly apply the webrev without creating a branch or commit.
>
> Currently the following options are supported:
>
> usage: git wimport [options] <webrev url|file path>
> -n, --name NAME Use NAME as a name for the patch (default is patch file name)
> -f, --file Input is a file path
> -k, --keep Keep downloaded patch file in current directory
> -d, --direct Directly apply patch, without creating a branch or commit
>
> ----------------
>
> Commits:
> - 7b20ce75: Wimport prototype
>
> Pull request:
> http://git.openjdk.java.net/skara/pull/32
>
> Webrev:
> https://openjdk.github.io/cr/skara/32/webrev.00
>
> Patch:
> http://git.openjdk.java.net/skara/pull/32.diff
>
> Fetch command:
> git fetch https://github.com/openjdk/skara.git pull/32/head:pull/32
args/src/main/java/org/openjdk/skara/args/MultiCommandParser.java line 41:
> 40: if (defaults.size() != 1) {
> 41: throw new IllegalStateException("Expecting exactly one default command");
> 42: }
`IllegalArgumentException` instead? The `commands` are passed in :)
PR: https://git.openjdk.java.net/skara/pull/32
args/src/main/java/org/openjdk/skara/args/MultiCommandParser.java line 70:
> 69: } else {
> 70: return this::showUsage;
> 71: }
Shouldn't this return the default command if one is present? Otherwise how will just `git webrev` work?
PR: https://git.openjdk.java.net/skara/pull/32
args/src/main/java/org/openjdk/skara/args/MultiCommandParser.java line 61:
> 60: if (p != null) {
> 61: String[] forwardedArgs = Arrays.copyOfRange(args, 1, args.length);
> 62: return () -> p.main(forwardedArgs);
This is a nice place for `var` ;)
PR: https://git.openjdk.java.net/skara/pull/32
vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java line 940:
> 939: apply(patchFile, force);
> 940: //Files.delete(patchFile);
> 941: }
We should probably uncomment this line :)
PR: https://git.openjdk.java.net/skara/pull/32
More information about the skara-dev
mailing list