Looks like we do not have self-tests for smbcacls

[Uri Simchoni]

I have only made a shallow review of the code (will really try to make a
proper review but I have a lot on my plate now), and have some general
comments / suggestions:

1. The switch name. In principle, the --propagate-inheritance feature
propagates changes made to an ACL, so it doesn't propagate just
inheritance information, it propagates ACEs. Maybe just --propagate?

2. Usually we "do what Windows does(tm)". However in this case it seems
like "Windows" is not the Windows OS but rather Windows Explorer. I
think the documentation and / or commit message should make it clear
what the reference implementation (or other reference) is, in case there
are bug reports down the road.

3. Assuming this *is* a Windows Explorer look-alike, Windows
Explorer pops up a message if it fails to set the ACL of a file,
allowing the user to continue or abort. IMHO that would be useful here,
because changing a large tree without the option to continue would be
difficult. The program can output messages on files which failed.

4. In the docs, there's a modification saying the flags serve smbcacls -
well they serve the SMB server too when creating new files and folders
(I think they are an artifact of the NTFS file system, and Samba
emulates that behavior).

Thanks,
Uri.

On 08/03/2017 02:41 PM, Noel Power via samba-technical wrote:
> And another version.....
> 
> On 31/07/17 20:34, Andrew Bartlett wrote:
>> On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
>> wrote:
>>> Please find a new version of the patchset (with tests re-written in python)
>>> Noel
>> Thanks for writing these tests.  It would have been better however if
>> the tests used the standard test classes like BlackboxTestCase and the
>> assertions that go with it.  
>>
>> I know it will be a pile of pain to re-work the test (again!), but the
>> consistency really helps as others will copy and paste from the first
>> example they find.
>>
>> See for example python/samba/tests/blackbox/ndrdump.py
>>
>> (We used to have more of those, but SambaToolCmdTest took over for
>> Samba tests).
>>
>> Sorry,
>>
>> Andrew Bartlett
> 
>