mbox series

[0/4] clone: allow configurable default for -o/--origin

Message ID pull.727.git.1599848727.gitgitgadget@gmail.com
Headers show
Series clone: allow configurable default for -o/--origin | expand

Message

Elijah Newren via GitGitGadget Sept. 11, 2020, 6:25 p.m. UTC
Took another pass at supporting a configurable default for-o/--origin, this
time following Junio's suggestions from a previous approach as much as
possible [1]. Unfortunately, Johannes mentioned that --template can write
new config values that aren't automatically merged without re-calling 
git_config. There doesn't appear to be a way around this without rewriting
significant amounts of init and config logic across the codebase.

While this could have been v2 of the original patchset, it's diverged so
drastically from the original that it likely warrants its own root thread.
If that's not appropriate though, I'd be happy to restructure!

Thanks for all the advice Junio and Johannes! This feels significantly
better than my first attempt.

[1] 
https://lore.kernel.org/git/pull.710.git.1598456751674.gitgitgadget@gmail.com/
[2] https://github.com/gitgitgadget/git/pull/727#issuecomment-689740195

Sean Barag (4):
  clone: add tests for --template and some disallowed option pairs
  clone: call git_config before parse_options
  clone: validate --origin option before use
  clone: allow configurable default for `-o`/`--origin`

 Documentation/config.txt       |  2 +
 Documentation/config/clone.txt |  5 +++
 Documentation/git-clone.txt    |  5 ++-
 builtin/clone.c                | 65 ++++++++++++++++++++++++++-------
 t/t5606-clone-options.sh       | 67 ++++++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/config/clone.txt


base-commit: 54e85e7af1ac9e9a92888060d6811ae767fea1bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-727%2Fsjbarag%2Fclone_add-configurable-default-for--o-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-727/sjbarag/clone_add-configurable-default-for--o-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/727

Comments

Derrick Stolee Sept. 11, 2020, 7:25 p.m. UTC | #1
On 9/11/2020 2:25 PM, Sean Barag via GitGitGadget wrote:
> Took another pass at supporting a configurable default for-o/--origin, this
> time following Junio's suggestions from a previous approach as much as
> possible [1]. Unfortunately, Johannes mentioned that --template can write
> new config values that aren't automatically merged without re-calling 
> git_config. There doesn't appear to be a way around this without rewriting
> significant amounts of init and config logic across the codebase.

Thanks for this update. I like the feature quite a bit, and all
of my comments are about style and organization instead of
functional issues.

> While this could have been v2 of the original patchset, it's diverged s
> drastically from the original that it likely warrants its own root thread.
> If that's not appropriate though, I'd be happy to restructure!

I think it is fine to restart the thread if the range-diff is not helpful.

Thanks,
-Stolee
Junio C Hamano Sept. 11, 2020, 7:34 p.m. UTC | #2
"Sean Barag via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Took another pass at supporting a configurable default for-o/--origin, this
> time following Junio's suggestions from a previous approach as much as
> possible [1]. Unfortunately, Johannes mentioned that --template can write
> new config values that aren't automatically merged without re-calling 
> git_config. There doesn't appear to be a way around this without rewriting
> significant amounts of init and config logic across the codebase.
>
> While this could have been v2 of the original patchset, it's diverged so
> drastically from the original that it likely warrants its own root thread.
> If that's not appropriate though, I'd be happy to restructure!

I did wonder if this series came from the same motivation while
scanning messages in the mailbox and saw no "v2" in the subject, but
I am OK either way in a case like this.  

I DO appreciate this note to explain why it is not marked with "v2".