Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Justin Tobler <jltobler@gmail.com>]

On 26/02/04 02:31AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> [snip]
> 
> > +
> > +/*
> > + * A transaction may be started for an object database prior to writing new
> > + * objects via odb_transaction_begin(). These objects are not committed until
> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending
> > + * at a time.
> > + *
> > + * Each ODB source is expected to implement its own transaction handling.
> > + */
> >  struct odb_transaction;
> 
> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
> here and drop this line?

I assume you mean drop the typedef in favor of defining it directly in
the struct and thus removing the need for the forward declation. I kind
of like having a typedef for the function callback, but I don't feel too
strongly either way. I've ammended locally, but will hold off from
sending another version unless there is other feedback.

> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
> > +struct odb_transaction {
> > +	/* The ODB source the transaction is opened against. */
> > +	struct odb_source *source;
> > +
> > +	/* The ODB source specific callback invoked to commit a transaction. */
> > +	odb_transaction_commit_fn commit;
> > +};
> >
> >  /*
> >   * The object database encapsulates access to objects in a repository. It
> > --
> > 2.52.0.373.g68cb7f9e92
> 
> Just a question in general, is the idea to eventually also add support
> for {prepare, rollback} within odb transactions?

I'm not quite sure yet about "prepare", but certainly an "abort" or
"rollback" will be introduced in a followup series. This will be useful
as we expand ODB transaction usage to other operations that require the
ability to remove temporary objects such as remerge-diffs.

Thanks,
-Justin