Michael Haggerty wrote: > I am still unhappy with the approach of this series, for the reasons > that I explained earlier [1]. In short, I think that the abstraction > level is wrong. In my opinion, consumers of the refs API should barely > even have to *know* about reflogs, let alone implement reflog expiration > themselves.
Okay, now returning to the substance of this comment. This is revisiting themes from [3], so my opinions are probably not a surprise. I think that the API changes that you and Stefan are proposing are consistent and could both go in. You suggested refactoring expire_reflogs() to use a callback that decides what to expire. Then it doesn't have to care that the expiration happens by creating a new reflog and copying over the reflog entries that are not being expired. The result is a clean reflog expiration API. The ref-transaction-reflog series allows those low-level steps to be part of a ref transaction. Any ref backend (the current files-based backend or a future other one) would get a chance to reimplement those low-level steps, which are part of what happens during ref updates and reflog deletion. The goal is for all reflog updates to use the transaction API, so that new ref/reflog backends only need to implement the transaction functions. So *both* are making good changes, with different goals. The implementation of the reflog expiration API can use the ref transaction API. > Of course, reflog expiration *should* be done atomically. But that is > the business of the refs module; callers shouldn't have to do all the > complicated work of building the transaction themselves. I don't understand this comment. After the ref-transaction-reflog series, a transaction_update_ref() call still takes care of the corresponding reflog update without the caller having to worry about it. Thanks for looking it over, Jonathan > [1] http://thread.gmane.org/gmane.comp.version-control.git/259712/focus=259770 [3] http://thread.gmane.org/gmane.comp.version-control.git/259939/focus=259967 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
