> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Ivan
> Zhakov
> Sent: zaterdag 31 oktober 2015 18:10
> To: Bert Huijben <[email protected]>
> Cc: [email protected]
> Subject: Re: svn commit: r1711582 - in /subversion/trunk/subversion:
> libsvn_fs_fs/tree.c tests/libsvn_fs/fs-test.c
> > I don't think you want to backport this, but returning
> SVN_ERR_FS_NOT_FILE
> > would make more sense here, than returning SVN_ERR_FS_GENERAL.
> >
> I agree that SVN_ERR_FS_NOT_FILE makes more sense. I'll do it.
>
> > And I would like to see the regression test checking that for all filesystem
> > layers for 1.10... but that is a bigger change.
> >
> We do not document this behavior, so I think it would be incorrect to
> require specific error code for
> svn_fs_contents_changed()/svn_fs_contents_different().
No, but as we go we can make our own implementations more strictly defined than
before.
(Our regression tests are for our implementations, aren't they?... Or should we
limit what we test based on theoretical third party filesystems?)
One of the reasons for not strictly documenting things before, might be that we
want to improve things later. Not documenting things gives more freedom to
change things for different reasons. (The whole C standard is based on
explicitly keeping things undefined).
The old code might have relied on falling back to error handling on a different
layer, which might have given a different (or more generic) error. But as we
already documented these two functions to only apply to files, there are good
reasons to test implementations on this... in fact you just added the test.
Once all implementations match a stricter standard we can improve the
documentation and implementation by producing a more stable and guaranteed
error.
(I'm hoping BDB already produces something sane)
Bert
