On 07/29/2016 09:38 PM, Lukas Slebodnik wrote:
> On (29/07/16 15:54), Michal Židek wrote:
>> On 07/29/2016 03:23 PM, Jakub Hrozek wrote:
>>> On Fri, Jul 29, 2016 at 03:06:47PM +0200, Lukas Slebodnik wrote:
>>>> On (29/07/16 14:27), Jakub Hrozek wrote:
>>>>> On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
>>>>>> On (29/07/16 13:59), Jakub Hrozek wrote:
>>>>>>> On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik
wrote:
>>>>>>>> On (29/07/16 13:44), Jakub Hrozek wrote:
>>>>>>>>> On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas
Slebodnik wrote:
>>>>>>>>>>> Others who? :-)
>>>>>>>>>> non developers (The person who requested this
change; I
>>>>>>>>>> assume this
>>>>>>>>>> change was not requested by developers)
>>>>>>>>>
>>>>>>>>> It was (and btw I agree with the change, consistent
naming is
>>>>>>>>> important
>>>>>>>>> as I wish I raised this concern when I reviewed the
patches in
>>>>>>>>> the first
>>>>>>>>> place..)
>>>>>>>> I was expecting an answer for keeping backward
compatibility with
>>>>>>>> unused feature.
>>>>>>>
>>>>>>> At this point it would be only compatibility for rawhide
users
>>>>>>> and anyone
>>>>>>> who compiled sssd from source or anyone who was alrady using
the
>>>>>>> 1.14.0
>>>>>>> tarball. Which is not many people, but still.
>>>>>> So if you want to keep old versions then
>>>>>> we document obsoleted version.
>>>>>> At least with help "Deprecated alias for (new-name)"
>>>>>>
>>>>>> e.g.
>>>>>> [root@host ~]# sssctl
>>>>>> Usage:
>>>>>> sssctl COMMAND COMMAND-ARGS
>>>>>>
>>>>>> Available commands:
>>>>>>
>>>>>> SSSD Status:
>>>>>> * list-domains Deprecated alias for (domain-list)
>>>>>> * domain-list List available domains
>>>>>> * domain-status Print information about domain
>>>>>>
>>>>>> The idea of hidding options is really terrible.
>>>>>> a) it's not documented anywhere that it's deprecated
>>>>>> b) users might wonder why it works.
>>>>>
>>>>> Fine by me, but additioanlly, what about printing the deprecation
>>>>> warning when a user runs that command?
>>>> If we really want to insist on "backward compatibility" with
unused
>>>> feature then it will be good addition to the the updated help output.
>>>>
>>>>> The point of hiding the option is
>>>>> to make it less discoverable.
>>>> I know what is point of hiding the option but it isn't good from
>>>> user point of view if we want to keep backward compatibility.
>>>> Backward compatible changes are usually well documented and not hiden
>>>>
>>>> Please update design page with renamed commands and also with
>>>> deprecated commands.
>>>
>>> OK, but this is something for the author of the patch to do :)
>>
>> Ok, I will update the design page, but I am little lost
>> in this thread. I will do this:
>>
>> 1. update the design page
> yes + ask for blessing/review from the requester of the change
>
>> 2. wrap the old command names in function that prints warning
>> about the command being deprecated and that it will be
>> removed in future versions.
>> 3. Will list the old commands in the sssctl usage with note
>> that they are deprecated.
>>
>> Do all agree with the above?
>>
> If we insinst on keeping backward comaptibility with unused
> feature then yes for 2) and 3)
>
> I would personally just rename them but I'm fine with 2) and 3)
> It would be much simpler.
>
> LS
CCing Marc.
Marc could you give us your opinion/blessing to this
change? Long story short, we though the commands will be
easier to understand if they follow similar pattern
as the TOPIC-ACTION that ipa tool uses. This will help
us create more consistent command names as we add more
functionality to the sssctl tool.
Using the obsolete commands is still possible and prints
message like this:
# sssctl fetch-logs out.tgz
Command 'fetch-logs' is deprecated and may be removed in future
versions. Use 'logs-fetch' instead.
We can remove the obsolete commands in some later milestone
o get rid of the code.
The usage now looks like this:
<USAGE>
# sssctl --help
Usage:
sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
* domain-list List available domains
* domain-status Print information about domain
Information about cached content:
* user-show Information about cached user
* group-show Information about cached group
* netgroup-show Information about cached netgroup
Local data tools:
* client-data-backup Backup local data
* client-data-restore Restore local data from backup
* cache-remove Backup local data and remove cached content
* cache-upgrade Perform cache upgrade
Log files tools:
* logs-remove Remove existing SSSD log files
* logs-fetch Archive SSSD log files in tarball
Configuration files tools:
* config-check Perform static analysis of SSSD configuration
Deprecated commands that may be removed in the future:
* list-domains Obsolete alias for domain-list
* user Obsolete alias for user-show
* group Obsolete alias for group-show
* netgroup Obsolete alias for netgroup-show
* backup-local-data Obsolete alias for client-data-backup
* restore-local-data Obsolete alias for client-data-restore
* remove-cache Obsolete alias for cache-remove
* upgrade-cache Obsolete alias for cache-upgrade
* remove-logs Obsolete alias for logs-remove
* fetch-logs Obsolete alias for logs-fetch
Common options:
--debug=INT The debug level to run with
Help options:
-?, --help Show this for a command
--usage Show brief usage message for a command
</USAGE>
I also amended commands mentioned in the design page,
to use the TOPIC-ACTION pattern. (For example debug-level
was changed to logs-level, and belongs to the same topic
as logs-remove and logs-fetch).
Link to desing page:
https://fedorahosted.org/sssd/wiki/DesignDocs/sssctl
Michal
Marc responded to me off list that he is also in favor
of the TOPIC-ACTION like pattern.
Marc's reaction:
<quote>
I vote for changing the options to the TOPIC-ACTION format.
Even if it felt different at the beginning when started using IPA, I
meanwhile like it. For me the options are easier to remember and it
allows to group them in the help, like in "ipa help TOPIC".
Regards,
Marc
</quote>