Matthieu Moy <[EMAIL PROTECTED]> writes:
> Stephen Leake <[EMAIL PROTECTED]> writes:
>
>> As an example, let's consider dvc-status. Currently, it looks like this:
>>
>> [...]
>
> Good example. I think we've mentionned that already, and we agree that
> the code can be factored better.
Ok.
>> So, taking things to the extreme, we _could_ refactor this:
>
> That's one step too far IMO: this doesn't allow a back-end to override
> status.
Do we have a use case where a back-end needs to override dvc-status
(or dvc-diff) _entirely_?
We have one for overriding dvc-diff-mode, in xgit, but that's because
it is a clean way to extend the menu and keymap.
Why would a back-end need to _override_ dvc-status, instead of
just defining a different function?
> I agree that providing a default implementation in DVC itself is
> good, but we should still allow more back-end specific stuff, in
> case.
What would your "favorite refactoring" look like?
>> Of course, we also need to consider what the other back-ends do. tla
>> handles nested trees; I'll just ignore that for now (we've agreed DVC
>> doesn't handle nested trees).
>
> No, you misunderstood me.
Ok.
> Having a tree whose root is below the root of another is good. tla has
> a few features to deal with that, git has submodule support, hg has
> the forest extension, bzr has the beginning of a by-reference nested
> tree support too (IIRC, it's implemented, but the commands are hidden
> because the bzr developpers are not satifsied with the UI yet. It
> might have changed since last time I checked). Nested trees are _the_
> way to deal with very large trees (like KDE or the FreeBSD ports) with
> any modern VCS.
Ok.
> BUT, the way you want to use nested trees is bad, most VCS won't even
> support it, and I've never seen any decent VCS user recommand it.
No, you misunderstood me.
> The difference is that you propose to have some files versionned both
> in the nested tree and in the containing tree. As opposed to that, any
> sane VCS will consider the nested tree as a black box, and never look
> at its files.
No. I was suggesting two different organizations, in two different
VCS's.
In bzr:
root
.bzr
dir_1
dir_2
sub_project
.bzr
dir_3
This works as you describe above.
In mtn:
root
_MTN
dir_1
dir_2
sub_project
dir_3
There is no _MTN in sub_project. That's just because I don't need
"sub_project" as a reusable project in _MTN.
But none of this was really important.
So DVC needs to support nested trees; it currently does not.
>> dvc-ignore-file-extensions, dvc-ignore-file-extensions-in-dir already
>> do things in this style.
>
> Yes, I like the way it's done.
Ok, good.
> It could be more generic by allowing direct back-end
> re-implementation, but since I have no use-case where this would be
> needed, it can/should remain as it is until we have a real use-case.
I don't see how it could both allow (total) back-end re-implementation,
_and_ reduce duplicated code; they are conflicting goals.
I agree we have no use case for that now.
>> Doing things this way accomplishes a couple of things:
>>
>> 1) Minimize maintenance complexity by reducing duplicate code among
>> back-ends.
>> [...]
>>
>> 2) Maxmize commonality of behavior across back-ends; all back-ends use
>> the same mode and buffer names for the status buffer, errors are
>> handled in the same way, etc. The DVC UI is more standardized.
>>
>> The importance of this goal is less widely accepted.
>
> I do agree with both.
Ok, good.
>> Another downside is that if we run into a new back-end that doesn't
>> fit the latest factorization, we might have to change things around
>> again. I think that's a risk worth taking; we have enough back-ends
>> now to come up with a reasonable factorization.
>
> Allowing a back-end to override dvc-status is actually quite easy. the
> `dvc-function'/`dvc-apply' stuff already allow a default fallback
> implementation.
Only if we _don't_ take the approach I'm advocating!
This is the approach dvc-status takes now; it _requires_ duplicated
code, and allows different behaviors.
> So, all you have to do is to implement status in dvc-dvc-status, and
> have dvc-status be a dispatching function (define-dvc-unified-command
> can probably be used for that). You have ~10 lines of code and O(1)
> overhead, for a good flexibility gain. That said, this change can be
> done later, without touching the back-ends.
I don't follow. There cannot be a default implementation of
dvc-status, since there is no default back-end command that makes
sense.
If the back-end has to provide _all_ of dvc-status (as required by
define-dvc-unified-command), then we have _not_ reduced duplicated code.
If the back-end has to provide _all_ of dvc-status, then having common
behavior relies on updating duplicated code; that is already not
happening.
--
-- Stephe
_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev
