Re: [PATCH 02/12] reftable/merged: make `merged_iter` structure private
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 27, 2024, 16:49 UTC
- Message-ID
- <Zd4Ss37Yl4d-oEzE@ncase>
- In-Reply-To
- <qprofop624nevbicid4rplfqlanfrujyxiilfqwchrppnmw7u4@3dvw5udjtrah>
On Tue, Feb 20, 2024 at 12:15:23PM -0600, Justin Tobler wrote:
Show 23 quoted lines
> On 24/02/14 08:45AM, Patrick Steinhardt wrote: > > The `merged_iter` structure is not used anywhere outside of "merged.c", > > but is declared in its header. Move it into the code file so that it is > > clear that its implementation details are never exposed to anything. > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > --- > > reftable/merged.c | 9 +++++++++ > > reftable/merged.h | 9 --------- > > 2 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/reftable/merged.c b/reftable/merged.c > > index 1aa6cd31b7..12ebd732e8 100644 > > --- a/reftable/merged.c > > +++ b/reftable/merged.c > > @@ -17,6 +17,15 @@ license that can be found in the LICENSE file or at > > #include "reftable-error.h" > > #include "system.h" > > > > suggestion: I think it would be nice to document a little about the > merge iterator here at a high-level. Maybe just to explain that this > allows iteration over multiple tables as if it were a single table.
Agreed. I have planned to invest more time into documenting the reftable library overall, but would rather want to push this out to another patch series.
Show 23 quoted lines
> > +struct merged_iter {
> > + struct reftable_iterator *stack;
> > + uint32_t hash_id;
> > + size_t stack_len;
> > + uint8_t typ;
> > + int suppress_deletions;
> > + struct merged_iter_pqueue pq;
> > +};
> > +
> > static int merged_iter_init(struct merged_iter *mi)
> > {
> > for (size_t i = 0; i < mi->stack_len; i++) {
> > diff --git a/reftable/merged.h b/reftable/merged.h
> > index 7d9f95d27e..288ad66656 100644
> > --- a/reftable/merged.h
> > +++ b/reftable/merged.h
> > @@ -24,15 +24,6 @@ struct reftable_merged_table {
> > uint64_t max;
> > };
> >
>
> Since we are removing `merge_iter` from the header here, I think we can
> also remove the `#include "pg.h"`.Good catch! We can replace it with "system.h", which makes sure to include <git-compat-util.h>.
Patrick