On 03/01, Rohit Ashiwal wrote: > Hey! > > I'm a little confused as you never provide a clear indication to > where shall I proceed? :- > > On Fri, 01 Mar 2019 11:51:46 +0900 Junio C Hamano <gitster@xxxxxxxxx> wrote: > > I think you are talking about t3600, which uses an ancient style. > > If this were a real project, then the preferred order would be > > > > - A preliminary patch (or a series of patches) that modernizes > > existing tests in t3600. Just style updates and adding or > > removing nothing else. > > > > - Update test that use "test -f" and friends to use the helpers in > > t3600. > > > > Yes, this is a microproject after all. But I think I can work on this as > if it were a real project, should I proceed according to this plan? (I have > a lot of free time over this weekend) Yes, I think it would be good to make those changes, to try and get this merged. Having the microproject merged is not a requirement (its main purpose is to see how students communicate on the mailing list, and to get them familiar with the workflow ahead of GSoC), but it can be a nice achievement in itself. That said, I would also encourage you to start thinking about a project proposal, as that is another important part that should be done for the application. > > > > If we often see if a path is an non-empty file in our tests (not > > limited to t3600), then it may make sense to add a new helper > > test_path_is_non_empty_file in t/test-lib-functions.sh next to where > > test_path_is_file and friends are defined. > > > > Since my project does not deal with `test-lib-functions.sh`, I think I > should not edit it anyway, but I'd be more than happy to add a new > member to `test_path_is_*` family. It is up to you how far you would like to go with this. If you want to add the helper, to make further cleanups in t3600, I think that would be a good thing to do (after double checking that it would be useful in other test files as well), and should be done in a separate patch. Then you can use it in the same patch as using the helpers for "test -f" etc. > Thanks > Rohit
