Hi,

On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote:
> Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt:
> > I still do not understand how the current behaviour is mismatching with
> > users expectations. Let's assume that you directly tracked the files of
> > L in your product repository P, without any submodule boundary. How
> > would the behavior be different? Would it be? If D started on an older
> > revision and gets merged into a newer revision, there can always be
> > regressions even without submodules.
> > 
> > Why would the core developer need to be informed about mismatching
> > revisions if he himself advanced the submodule?
> In that case you'd be right. I should have picked my example more wisely.
> Assume right here that not a core developer, but another developer advanced
> the submodule (also via feature branch + merge).
> > 
> > It seems to me that you do not want to mix integration testing and
> > testing of the feature itself. 
> That's on point. That's why it would be nice if git *at least* warned
> about the different revisions wrt submodules.
> 
> But, I guess, I learned something about submodules:
> I used to think of submodules as means to pin down a specific revision like: 
> `ver == x`.
> Now I'm learning that submodules are treated as `ver >= x` during a merge.

Well a submodule version is pinned down as long a you do not change it
and commit it. The same as files and the goal is to make submodules
behave as close to normal files as possible. And git "warns" about
changed submodules by displaying them in the diff.

Actually the use case you are describing is not even involving a real
merge for submodules. It is just changing the pointer to another
revision.

> > How about just testing/reviewing on the
> > branch then? You would still get the submodule revision D was working on
> > and then in a later stage check if integration with everything else
> > works.
> Sure. But if the behavior deviates after a merge the merging developer is 
> currently not
> aware that it *might* have to do with different submodule revisions used, not 
> the "actual" code merged.
> 
> Like not even "beware: the (feature) branch you've merged used an 'older' 
> revision of X"

The submodule is part of the "actual" code and should be reviewed the
same. Maybe you want to set the diff.submodule option to 'diff' ? Then
git shows the actual diff of the changed contents in the submodule and
it would be more obvious how the code changed.

At the moment it seems to me that you want submodules to behave
differently than we handle normal files/directories which is the
opposite direction we have been trying to get git into. My feeling
though is that this should be covered by the review process instead of a
failing merge. Another option would be that you could write a hook that
warns reviewers that they are merging a submodule update.

Cheers Heiko

Reply via email to