From: Robert Coup Date: Wed, 09 Mar 2022 09:57:37 GMT Subject: Re: [PATCH v3 0/7] fetch: add repair: full refetch without negotiation (was: "refiltering") Message-ID: In-Reply-To: <20220309002729.3581315-1-calvinwan@google.com> Hi Calvin, On Wed, 9 Mar 2022 at 00:27, Calvin Wan wrote: > > This is my first time sending a review to the list, so forgive me for any > mistakes I make or conventions missed. Feedback about my review would be well > appreciated! I'm as new as you to contributing to Git :-) > Overall I think your patch is well written and the implementation accomplishes > what you describe in your cover letter, Thanks! I think if you're completely content you can add your Reviewed-By as described at https://git-scm.com/docs/SubmittingPatches#commit-trailers > I would like to discuss another > possible design I thought of. Currently, the user has to know to run ‘--refetch’ > after changing the partial clone filter configuration in order to fetch the > commits that match the new filter. Ideally I believe this behavior should be > default so therefore instead of adding an option, if git knew what filter was > last used in the fetch, it could automatically ‘refetch’ everything if there is > a change between the last used filter and the default filter. So, if you do a partial clone using `git clone --filter=...` then the filter is saved into the config at `remote..partialclonefilter` and is re-used by default for subsequent fetches from that remote. But there's nothing to stop `git fetch --filter=...` being run multiple times with different filters to carefully setup a repository for a particular use case, or any notion that there has to be "one" filter in place for a remote. Running `git fetch --filter=...` doesn't update the remote's partial clone filter in the config, and IMO it shouldn't for the above reason. I think having `git config remote..partialclonefilter ` print out something to the user along the lines of "Your change to/removal of the filter won't fetch in additional objects associated with existing commits, you can do this with `fetch --refetch `" could be helpful, but after a very quick look I can't see anything like that at the moment for other config settings (ie. no plumbing in place to easily reuse), and I'm not motivated to add such plumbing. Cheers, Rob :)