[ignoring-somethings] Improve verify logic

[SAN-MUYUN]

Exercise Review

Exercise Discussion

#226

Checklist

• If you require a new remote repository on the Git-Mastery organization, have you created a request for it?
• Have you written unit tests using repo-smith to validate the exercise grading scheme?
• Have you tested your changes using the instructions posted?
• Have you verified that this exercise does not already exist or is not currently in review?
• Did you introduce a new grading mechanism that should belong to git-autograder?
• Did you introduce a new dependency that should belong to app?

[github-actions]

Hi @SAN-MUYUN, thank you for your contribution! 🎉

This PR comes from your fork SAN-MUYUN/git-mastery-exercises on branch enhancement/ignoring-somethings.

Before you request for a review, please ensure that you have tested your changes locally!

Important

The previously recommended way of using ./test-download.py is no longer the best way to test your changes locally.

Please read the following instructions for the latest instructions.

Prerequisites

Ensure that you have the gitmastery app installed locally (instructions)

Testing steps

If you already have a local Git-Mastery root to test, you can skip the following step.

Create a Git-Mastery root locally:

gitmastery setup

Navigate into the Git-Mastery root (defaults to gitmastery-exercises/):

cd gitmastery-exercises/

Edit the .gitmastery.json configuration file. You need to set the following values under the exercises_source key.

{
    # other fields...
    "exercises_source": {
        "username": "SAN-MUYUN",
        "repository": "git-mastery-exercises",
        "branch": "enhancement/ignoring-somethings",
    }
}

Then, you can use the gitmastery app to download and verify your changes locally.

gitmastery download <your new change>
gitmastery verify

Checklist

• (For exercises and hands-ons) I have verified that the downloading behavior works
• (For exercises only) I have verified that the verification behavior is accurate

Important

To any reviewers of this pull request, please use the same instructions above to test the changes.

[VikramGoyal23]

Mostly LGTM, just some minor nits! Also since, I couldn't post it directly in the code, we should consider changing MISSING_COMMITS to You have not committed the relevant changes yet! since the user could have made a commit and changed the .gitignore again, causing them to receive the message despite making the some commits. This allows us to provide helpful feedback for two scenarios at once (Making no changes or commits OR having a dirty tree, i.e., uncommitted changes to .gitignore, which both lead to MISSING_COMMITS being added to feedback).

[VikramGoyal23]

The convention for formatting is to leave 2 lines of whitespace before function definitions.

[VikramGoyal23]

The committed .gitignore is unlikely to be None unless the student deletes the file and then commits that deletion, before then remaking the file. However, since this is completely possible and we don't have any other error handling for this scenario, this line is a good safety barrier.

[VikramGoyal23]

Could you move the definition of no_user_commit to after line 45? Just to decrease its active scope.

[SAN-MUYUN]

@VikramGoyal23 Thanks for the feedback! Updated accordingly

[jovnc]

@desmondwong1215 @SAN-MUYUN Also do include closing words like "fix #234" next time when opening PR so that it will link to the issue automatically

[VikramGoyal23]

LGTM, tested it and it works!

[desmondwong1215]

@desmondwong1215 @SAN-MUYUN Also do include closing words like "fix #234" next time when opening PR so that it will link to the issue automatically

okk noted