Hi Joey
On 20/10/2020 08:24, Joey S wrote:
Hi everyone,
Thank you very much for the input and feedback, it's much appreciated.
All this text above is useful context for reviewers but appears as part
of the commit message which is not what you want. If you add notes after
the `---` line below then they will not end up in the commit message.
Understood, thank you.
Modernized the test by replacing 'test -e' instances with
test_path_is_file helper functions.
s/Modernized/Modernize/
Will do in the amended commit next.
- ! test_path_is_file paginated.out
It would be better to replace`! test -e` this with
`test_path_is_missing` as the modified test will pass if paginated.out
exists but is not a file. `test_path_is_missing` will print an
appropriate diagnostic message as well.
Thank you for the explanation : )
After replacing `! test -e` with `! test_path_is_missing paginated.out` however, the changed test cases are failing;
```
$ cd t/ && prove t7006-pager.sht7006-pager.sh .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 3/101 subtests
Test Summary Report
-------------------
t7006-pager.sh (Wstat: 256 Tests: 101 Failed: 3)
Failed tests: 7-9
Non-zero exit status: 1
Files=1, Tests=101, 5 wallclock secs ( 0.03 usr 0.00 sys + 3.49 cusr 0.65 csys = 4.17 CPU)
Result: FAIL
```
Is this the behavior I should be expecting?
No it's not! As one aspect of this process is to help candidates improve
their understanding I'll give you a hint rather than the whole answer.
`test -e <path>` checks whether <path> exists and exits 0 if it does and
the shell treats an exit code of 0 as success. `!` inverts the
success/failure of the command that follows it. Using that and looking
at the definition of test_file_is_missing in t/test-lib-functions.sh see
if you can fix the conversion so that the tests pass. If you get stuck
do let me know and I'll try and help some more.
Best Wishes
Phillip
...Alternatively, this would fit just fine in a cover letter. Usually
cover letters are not necessary for single patches (where the patch
message itself conveys the full message, or a little bit of additional
context below the triple-dash line is all that's necessary to clarify
the intent). But, if you want to introduce yourself, a 0/1 cover letter
is fine, too.
Will keep this in mind, thank you Taylor.
One thing missed by other commenters: the Developer's Certificate of
Origin line - which is what this indicates - should have your "full
name".
... and it must match the authorship.
Changed, thank you both for catching that.
Thank you all,
Joey
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, October 19, 2020 7:59 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
Emily Shaffer emilyshaffer@xxxxxxxxxx writes:
On Mon, Oct 19, 2020 at 04:26:07AM +0000, Joey S wrote:
Hi everyone,
Hi Joey and welcome.
Signed-off-by: JoeyS jgsal@xxxxxxxxx
One thing missed by other commenters: the Developer's Certificate of
Origin line - which is what this indicates - should have your "full
name".
... and it must match the authorship.
So in my case, I sign my patches 'Emily Shaffer
emilyshaffer@xxxxxxxxxx'. If I'm wrong that's fine, but JoeyS sounds
like a name and initial rather than a full name.
Thanks for pointing it out.
If somebody from the "mentoring" group is taking a tally, it might
not be a bad idea to identify which style and procedure rules are
often failed to be followed by new contributors so that we can
figure out ways to make them stand out in our documentation set
(e.g. Documentation/SubmittingPatches but maybe a separate cheat
sheet might be worth having).
Thanks.
