Code review comment for lp:~piastucki/bzr-xmloutput/xml-shelve-list

Revision history for this message
Piotr Piastucki (piastucki) wrote :

> The code looks good.
> Just one question, any reason to add the specific xmlshelvelist command
> instead of a command + option like the shelve command, e.g: xmlshelve --list?
>
> Thanks

Good point. I was also thinking about that. I decided to add a new command as the main purpose of the shelve command is shelving files and --list option seems to be secondary. I think it is a good idea to provide XML versions of the commands that generate complex output and avoid re-implementing commands that modify anything. In this case I could add xmlshelve command that would support only --list option and throw an error otherwise. (or maybe delegate to built-in commands instead?). Instead of adding a very limited xmlshelve command I added xmlshelvelist to make it more obvious what the command actually does. However, I am not fully convinced this is the best apporach either. Please let me know what you think and I will change the code accordingly.

« Back to merge proposal