Skip to content

[tooling] Update set up scripts#277

Open
jovnc wants to merge 1 commit intogit-mastery:mainfrom
jovnc:chore/update-setup-script
Open

[tooling] Update set up scripts#277
jovnc wants to merge 1 commit intogit-mastery:mainfrom
jovnc:chore/update-setup-script

Conversation

@jovnc
Copy link
Copy Markdown
Collaborator

@jovnc jovnc commented Mar 27, 2026

Summary

Update set up scripts to remove unnecessary code in boilerplate for hands on and exercises

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 27, 2026

Hi @jovnc, thank you for your contribution! 🎉

This PR comes from your fork jovnc/exercises on branch chore/update-setup-script.

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": "jovnc",
        "repository": "exercises",
        "branch": "chore/update-setup-script"
    }
}

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.

Copy link
Copy Markdown
Collaborator

@VikramGoyal23 VikramGoyal23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but IMO the import commands at the top help enforce the programming style a bit. Should we consider leaving the run_command import(s)? Other changes are good.

@jovnc
Copy link
Copy Markdown
Collaborator Author

jovnc commented Mar 27, 2026

We shouldn't be encouraging the use of run_command, we already have built-in functions, and contributors should only turn to run_command as last resort

@jovnc
Copy link
Copy Markdown
Collaborator Author

jovnc commented Mar 27, 2026

Coding style is no problem, as we have linters and formatters in place

Copy link
Copy Markdown
Contributor

@jiaxinnns jiaxinnns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that run_command should be avoided and the import can mislead new developers into thinking otherwise. LGTM

@VikramGoyal23
Copy link
Copy Markdown
Collaborator

I was referring to the style of import (from X import Y rather than import X[ as Z]). Ruff does not enforce this distinction. I do agree that we can potentially find another way to inform developers of this instead of putting specifically run_command as the imported function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants