Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> writes: > 1. Commit message was less than 50 chars which should be around 72 chars > according to coding guide lines. Should I change this to match 72? Simple things do not need that many letters to tell ;-) The suggestion of 72 is about the maximum. If you are doing something in a single patch that needs a longer title, it generally is a sign that you are trying to do too much in a single patch and should be splitting the patch into more digestable smaller steps. And the purpose of having a maximum is to nudge patch authors to realize that. > 2. My changes had some uneven use of tabs and spaces, which I made > considering that pre-existing code had them too. Is there a > possibility to change the whole code according to CodingGuidelines? > If yes should I only change my code according to guidelines or the > whole file? 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. > 3. There is no helper function for `test -s` but Rafael suggested we can > make use of other helper functions to provide similar functionality, > if we can. 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. Thanks. [jch: I am still mostly offline til the next week, but I had a chance to sit in front of my mailbox long enough, so...]
