From: bilalobe <elkhatabibilal@gmail.com>

Replace an assertion-style `test -f` check with `test_path_is_file` in `t/t5315-pack-objects-compression.sh`.

This aligns the test with the path-checking helpers used in Git's test suite.

Found with:
  git grep "test -[efd]" t/

Signed-off-by: bilalobe <elkhatabibilal@gmail.com>
---
 t/t5315-pack-objects-compression.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Bilal El Khatabi (<elkhatabibilal@gmail.com>) writes:

I see this is sent from Bilal El Khatabi and From has a different
name, How do you want to be known in the community?
PS: from Documentation/SubmittingPatches:
"It is common, but not required, to use some form of your real name.
We realize that some contributors are not comfortable doing so or
prefer to contribute under a pseudonym or preferred name and we can
accept your patch either way, as long as the name and email you use
are distinctive, identifying, and not misleading."

Try explaining why is this change needed

It would help to explain why this is better than what was before, for example that this helper reports loudly what expectation wasn't met, therefore makes it easier to debug.

Many are adding this prob because it is said on the microproject to add the command you used to find the file, but because this is just one instance, it's not very useful. It's better to drop it.

Seeing that this is a microproject and the current date, if this is for GSoC, add to the subject GSoC with PATCH "[GSoC PATCH]" for example, and CC your possible co mentors.

Code seems fine

Use test_path_is_file instead of test -f when checking that the loose object was written to the expected path.

This uses Git's path-checking helper, which provides more specific failure output than a raw test -f check.

Signed-off-by: Bilal El Khatabi <elkhatabibilal@gmail.com>
---
 t/t5315-pack-objects-compression.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Pablo <pabloosabaterr@gmail.com> writes:

Thanks for noting these things for new folks. Another thing to encourage is to read and be aware of what other new folks in the community has done recently by reading

    https://lore.kernel.org/git/

and at least skim a handful of threads, and a few of them that are titled similarly to what they are about to post in detail.

Thanks.

Bilal El Khatabi <elkhatabibilal@gmail.com> writes:

These two sentences repeat almost the same thing in different phrasing, which starts to become a bit boring.

The gold standard way to write a proposed commit log message for this project is to:

 - Give an observation on how the current system works in the
   present tense (so no need to say "Currently X is Y", or
   "Previously X was Y" to describe the state before your change;
   just "X is Y" is enough), and discuss what you perceive as a
   problem in it.

 - Propose a solution (optional---often, problem description
   trivially leads to an obvious solution in reader's minds).

 - Give commands to somebody editing the codebase to "make it so",
   instead of saying "This commit does X".

in this order. The above jumps directly to the execution, bypassing the observation and solution. Fully followed, it would become something like:

    When a test in t5315 tries to see if a loose object file is
    created at the expected path in the filesystem, "test -f" is
    used.  If a developer breaks "git" in such a way that the file
    is no longer created at this expected place, however, "test -f"
    will silently fail and exits with non-zero, causing the test
    fail, but it is not obvious which step in the &&- chained
    commands failed.

    Use test_path_is_file helper, which loudly reports the failure
    when the expectation is not met.  When running the test with the
    "-v" option, i.e., "sh t5315-*.sh -v", the failure would become
    more prominent, helping the developer.

Will queue.

Thanks, Junio and Pablo.

  For the advice, will do.

  I’ll also spend some time reading recent newcomer and GSoC-related
  threads on lore before sending the next ones.

Best, Bilal

BILAL EL KHATABI <elkhatabibilal@gmail.com> writes:

One tip is to not top-post when replying :)

From Documentation/MyFirstContribution.adoc, we have:

  The Git list requires plain-text-only emails and prefers inline and
  bottom-posting when replying to mail;

- Karthik