Hi Phillip On 2019-06-10 10:39 UTC Phillip Wood <phillip.wood123@xxxxxxxxx> wrote: > >Hi Rohit > >On 10/06/2019 06:13, Rohit Ashiwal wrote: >> >> [...] >> Firstly, signature of `create_seq_dir` doesn't allow us to call >> `sequencer_get_last_command()`. Changing that for the sake of a >> better error message is too much task for this patch as it is a >> subject of discussion on its own. > > There is only one caller and it already has a struct repository pointer > so it is a two line change, one of which is the insertion of a single > character to change create_seq_dir() so it can call > sequencer_get_last_command(). It is normal to change function signatures > (especially for static functions like this) when making changes, it is > part of improving the code base. The quality of error messages is > important to the overall user experience. It's when things go wrong that > users need accurate advice about what to do. Nice! I'll do this in next revision then. >> (Also changing signature only >> makes sense if this patch series gets merged). FWIW, I think we >> should left this to further discussions for now and decide what >> to do later on. > > It is only a small change so why not do it now rather than putting it > off for another series which will be more work in the long run. I'm fine with changing that here. Thanks Rohit
