On Wed, Oct 24, 2018 at 2:39 AM <nbelakovski@xxxxxxxxx> wrote: > lock_reason is now populated during the execution of get_worktrees > > is_worktree_locked has been simplified, renamed, and changed to internal > linkage. It is simplified to only return the lock reason (or NULL in case > there is no lock reason) and to not have any side effects on the inputs. > As such it made sense to rename it since it only returns the reason. > > Since this function was now being used to populate the worktree struct's > lock_reason field, it made sense to move the function to internal > linkage and have callers refer to the lock_reason field. The > lock_reason_valid field was removed since a NULL/non-NULL value of > lock_reason accomplishes the same effect. > > Some unused variables within worktree source code were removed. Thanks for the submission. One thing which isn't clear from this commit message is _why_ this change is desirable at this time, aside from the obvious simplification of the code and client interaction (or perhaps those are the _why_?). Although I had envisioned populating the "reason" field greedily in the way this patch does, not everyone agrees that doing so is desirable. In particular, Junio argued[1,2] for populating it lazily, which accounts for the current implementation. That's why I ask about the _why_ of this change since it will likely need to be justified in a such a way to convince Junio to change his mind. Thanks. [1]: https://public-inbox.org/git/xmqq8tyq5czn.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/ [2]: https://public-inbox.org/git/xmqq4m9d0w6v.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
