| Message ID | cover.1722316795.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | refs: stop using `the_repository` | expand |
Patrick Steinhardt <ps@pks.im> writes: > Hi, > > this patch series removes use of `the_repository` in the refs subsystem > and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those > files. > So the idea is to slowly cleanup usages of `the_repository` and the `USE_THE_REPOSITORY_VARIABLE` macro acts as a check for this. I think the changes look great. I even ran clang-format on this series and apart from line wrap suggestions, there were no issues. Thanks for this. [snip]
On Tue Jul 30, 2024 at 3:22 PM AEST, Patrick Steinhardt wrote: > Hi, > > this patch series removes use of `the_repository` in the refs subsystem > and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those > files. > > Patrick Thanks Patrick, these improvements make sense to me. Is there a priority order on removing `the_repository` from other parts of the codebase? Cheers, James
On Tue, Jul 30, 2024 at 07:26:50AM -0400, Karthik Nayak wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > Hi, > > > > this patch series removes use of `the_repository` in the refs subsystem > > and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those > > files. > > > > So the idea is to slowly cleanup usages of `the_repository` and the > `USE_THE_REPOSITORY_VARIABLE` macro acts as a check for this. I think > the changes look great. I even ran clang-format on this series and apart > from line wrap suggestions, there were no issues. Exactly. One intent is to demonstrate to reviewers that a file actually got rid of `the_repository`. The second intent is to hide away APIs that have an implicit dependency on `the_repository`, which would be easy to miss otherwise. The second part is not perfect because we only hide away a subset of interfaces. That part can be extended over time as required though. Patrick
On Wed, Jul 31, 2024 at 10:55:09AM +1000, James Liu wrote: > On Tue Jul 30, 2024 at 3:22 PM AEST, Patrick Steinhardt wrote: > > Hi, > > > > this patch series removes use of `the_repository` in the refs subsystem > > and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those > > files. > > > > Patrick > > Thanks Patrick, these improvements make sense to me. > > Is there a priority order on removing `the_repository` from other parts > of the codebase? I was tackling the refs API because I knew that it was a "leaf" package that doesn't have a ton of dependencies on subsystems that may be using `the_repository` itself. So I guess that the order that makes most sense is from leaf subsystems up to the root such that we can adjust layer by layer. Figuring out what those leaf subsystems are is a different story though. I typically tend to brute force it, see how far I get and if I succeed then I don't mention all the failed tries that led to the patch series ;) Over time you then get some intuition for which parts to handle next, even though I realize that this is not particularly useful as advice to somebody not that familiar with the codebase. Patrick
Hi, this patch series removes use of `the_repository` in the refs subsystem and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those files. Patrick Patrick Steinhardt (5): refs: stop using `the_repository` refs/files: stop using `the_repository` in `parse_loose_ref_contents()` refs/files: stop using `the_repository` refs/packed: stop using `the_repository` refs/reftable: stop using `the_repository` refs.c | 16 ++++++------- refs/files-backend.c | 29 ++++++++++++----------- refs/packed-backend.c | 14 +++++------ refs/refs-internal.h | 3 ++- refs/reftable-backend.c | 51 +++++++++++++++++++++-------------------- 5 files changed, 56 insertions(+), 57 deletions(-)