On Tue, Feb 11, 2020 at 01:08:48PM +1300, Heba Waly wrote: > On Tue, Feb 11, 2020 at 8:42 AM Taylor Blau <me@xxxxxxxxxxxx> wrote: > > > > On Mon, Feb 10, 2020 at 11:30:46AM -0800, Junio C Hamano wrote: > > > Another thing. > > > > > > advise_ng() may have been a good name for illustration but is a > > > horrible name for real-world use (imagine we need to revamp the API > > > one more time in the future---what would it be called, which has to > > > say that it is newer than the "next generation"? > > > advise_3rd_try()?). > > As I mentioned earlier, this patch is meant to be used as a transition > between the current advise() and the refactored one. Ah, thanks for pointing it out, and I'm sorry that I missed reading it in my first review. Your idea sounds quite good to me, and thanks for working on this. > So the name is just temporary and it'll be renamed to advise() once > the transition is done. > But if we want to keep both functions, or want a better name because > it's an open-source project and the author might not complete the > transition, then I'll try to think of another name. > > > What about calling this new API 'advise()'? The first patch could call > > it 'advise_ng' or whatever other temporary name we feel comfortable > > using, and then each subsequent patch would update callers of 'advise()' > > to use 'advise_ng()'. Once those patches have been applied, and no other > > callers of 'advise()' exist, a final patch can be applied on top to > > rename 'advise_ng()' to 'advise()', and update the names of all of the > > callers. > > > > Yes, that's what I would like to do. > > > This makes for a rather noisy final patch, but the intermediate states > > are much clearer, and it would make this series rather self-contained. > > > > On the other hand, having a version of 'advise_ng()' on master makes > > this topic more incremental, meaning that we can pick it up and put it > > down at ease and have more self-contained projects. > > > > I don't really have a preference between the two approaches, but if we > > go with the latter, I do think we need something better than > > 'advise_ng'. Maybe 'advise_warn'? I don't know. > > > > > Thanks. > > > > Thanks, > > Taylor > > Thanks, > Heba Thanks, Taylor
