mbox series

[0/2] Specify the actual pack size limit which is breached

Message ID pull.1158.git.1645632193.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Specify the actual pack size limit which is breached | expand

Message

Linus Arver via GitGitGadget Feb. 23, 2022, 4:03 p.m. UTC
Git allows configuring a maximum pack size. GitHub (like presumably other
Git hosts) configures this setting to a generous but finite limit. When a
user attempts to push an oversized pack, their connection is terminated with
a message that they've exceeded the limit. The user has to find the limit
value elsewhere, probably in the host's documentation. This change adds a
small convenience -- specifying the limit itself in the error message -- so
that users no longer have to search elsewhere to discover the limit.

Matt Cooper (2):
  index-pack: clarify the breached limit
  t5302: confirm that large packs mention limit

 builtin/index-pack.c  | 8 ++++++--
 t/t5302-pack-index.sh | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)


base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1158%2Fvtbassmatt%2Fmc%2Fhumanize-limit-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1158/vtbassmatt/mc/humanize-limit-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1158

Comments

Taylor Blau Feb. 23, 2022, 4:33 p.m. UTC | #1
On Wed, Feb 23, 2022 at 04:03:11PM +0000, Matt Cooper via GitGitGadget wrote:
> Matt Cooper (2):
>   index-pack: clarify the breached limit
>   t5302: confirm that large packs mention limit
>
>  builtin/index-pack.c  | 8 ++++++--
>  t/t5302-pack-index.sh | 8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)

I took a look and reviewed this series internally. The patches here
match what I looked at within GitHub, so these look good to me also.

For what it's worth, I wouldn't mind to see these two patches squashed
together, since it may be easier for future readers to see the new code
and test together in the same patch.

But I don't feel strongly about it, so (with or without that suggestion)
I'd be happy to see this get picked up. Thanks, Matt!

Thanks,
Taylor