Hi Rohit On 10/06/2019 14:25, Rohit Ashiwal wrote: > 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. I'm glad that helped > >>> (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. That's great Best Wishes Phillip > > Thanks > Rohit >
