Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation: Increase example cache timeout to 1 hour #1412

Closed
wants to merge 1 commit into from

Conversation

hickford
Copy link

@hickford hickford commented Nov 9, 2022

Previously, the example decreased the cache timeout compared to the
default, nudging users to make cache less usable.

Instead, nudge users to make cache more usable. Currently many users
choose store over cache for usability. See
https://lore.kernel.org/git/Y2p4rhiOphuOM0VQ@coredump.intra.peff.net/

The default timeout remains 15 minutes. A stronger nudge would
be to increase that.

Signed-off-by: M Hickford mirth.hickford@gmail.com

CC: Jeff King peff@peff.net
cc: Taylor Blau me@ttaylorr.com

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 9, 2022

There are issues in commit 30f8262:
Documentation: Increase example cache timeout to 1 hour
Prefixed commit message must be in lower case

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 9, 2022

There are issues in commit c11f094:
Documentation: Increase example cache timeout to 1 hour
Prefixed commit message must be in lower case

@hickford
Copy link
Author

hickford commented Nov 9, 2022

/submit

@hickford hickford marked this pull request as ready for review November 9, 2022 10:18
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 9, 2022

Submitted as pull.1412.git.1667989181611.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1412/hickford/nudge-cache-longer-v1

To fetch this version to local tag pr-1412/hickford/nudge-cache-longer-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1412/hickford/nudge-cache-longer-v1

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 9, 2022

On the Git mailing list, Jeff King wrote (reply to this):

On Wed, Nov 09, 2022 at 10:19:41AM +0000, M Hickford via GitGitGadget wrote:

> From: M Hickford <mirth.hickford@gmail.com>
> 
> Previously, the example *decreased* the cache timeout compared to the
> default, nudging users to make cache less usable.

I don't mind at all changing this as your patch does. The existing
example was mostly just to illustrate the syntax. But...

> Instead, nudge users to make cache more usable. Currently many users
> choose store over cache for usability. See
> https://lore.kernel.org/git/Y2p4rhiOphuOM0VQ@coredump.intra.peff.net/

I don't see how my email argues for this. The only thing I mentioned
about credential-cache there is that it's not available on all
platforms.

But if you want my opinion on its usability, the main problem is not
that the cache timeout. It's that entering the credential at all is a
pain, either because it's a semi-automated environment that needs to
operate without user input, or because the credential itself is awkward
for the user to enter (like a long token). And that's what pushes people
to "store" over "cache".

> diff --git a/Documentation/git-credential-cache.txt b/Documentation/git-credential-cache.txt
> index 0216c18ef80..432e159d952 100644
> --- a/Documentation/git-credential-cache.txt
> +++ b/Documentation/git-credential-cache.txt
> @@ -69,10 +69,10 @@ $ git push http://example.com/repo.git
>  ------------------------------------
>  
>  You can provide options via the credential.helper configuration
> -variable (this example drops the cache time to 5 minutes):
> +variable (this example increases the cache time to 1 hour):
>  
>  -------------------------------------------------------
> -$ git config credential.helper 'cache --timeout=300'
> +$ git config credential.helper 'cache --timeout=3600'
>  -------------------------------------------------------

The patch itself is obviously correct.

-Peff

Previously, the example *decreased* the cache timeout compared to the
default, making it less user friendly.

Instead, nudge users to make cache more usable. Many users choose
store over cache.
https://lore.kernel.org/git/CAGJzqskRYN49SeS8kSEN5-vbB_Jt1QvAV9QhS6zNuKh0u8wxPQ@mail.gmail.com/

The default timeout remains 15 minutes. A stronger nudge would
be to increase that.

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
@hickford
Copy link
Author

hickford commented Nov 9, 2022

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 9, 2022

Submitted as pull.1412.v2.git.1668010273573.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1412/hickford/nudge-cache-longer-v2

To fetch this version to local tag pr-1412/hickford/nudge-cache-longer-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1412/hickford/nudge-cache-longer-v2

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 9, 2022

On the Git mailing list, M Hickford wrote (reply to this):

On Wed, 9 Nov 2022 at 13:18, Jeff King <peff@peff.net> wrote:
>
> > Instead, nudge users to make cache more usable. Currently many users
> > choose store over cache for usability. See
> > https://lore.kernel.org/git/Y2p4rhiOphuOM0VQ@coredump.intra.peff.net/
>
> I don't see how my email argues for this. The only thing I mentioned
> about credential-cache there is that it's not available on all
> platforms.

I'll amend the commit message.

> But if you want my opinion on its usability, the main problem is not
> that the cache timeout. It's that entering the credential at all is a
> pain, either because it's a semi-automated environment that needs to
> operate without user input, or because the credential itself is awkward
> for the user to enter (like a long token). And that's what pushes people
> to "store" over "cache".

I agree. I have some ideas to help human users; I'll share in the other thread.

> The patch itself is obviously correct.

Thanks for the review

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 10, 2022

On the Git mailing list, Taylor Blau wrote (reply to this):

On Wed, Nov 09, 2022 at 04:09:14PM +0000, M Hickford wrote:
> > The patch itself is obviously correct.
>
> Thanks for the review

Thanks, both. I picked up the updated round.

Thanks,
Taylor

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 10, 2022

User Taylor Blau <me@ttaylorr.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 10, 2022

This branch is now known as mh/increase-credential-cache-timeout.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 10, 2022

This patch series was integrated into seen via git@24e4388.

@gitgitgadget gitgitgadget bot added the seen label Nov 10, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 11, 2022

This patch series was integrated into seen via git@76850f9.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 15, 2022

This patch series was integrated into seen via git@0cd07d5.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 15, 2022

This patch series was integrated into next via git@afe54db.

@gitgitgadget gitgitgadget bot added the next label Nov 15, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 15, 2022

This patch series was integrated into seen via git@645f482.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 15, 2022

This patch series was integrated into seen via git@be2fc89.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 15, 2022

This patch series was integrated into seen via git@86cacd7.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 16, 2022

This patch series was integrated into seen via git@7718b96.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 16, 2022

This patch series was integrated into seen via git@78fcc69.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 17, 2022

This patch series was integrated into seen via git@96b4656.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 19, 2022

This patch series was integrated into seen via git@3f98d7a.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 19, 2022

This patch series was integrated into master via git@3f98d7a.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 19, 2022

This patch series was integrated into next via git@3f98d7a.

@gitgitgadget gitgitgadget bot added the master label Nov 19, 2022
@gitgitgadget gitgitgadget bot closed this Nov 19, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 19, 2022

Closed via 3f98d7a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant