Classroom Glossary Public page

Lab 10: PR Submission

1,051 words

~90 minutes. Take Lab 9's disk-usage reporter; push to a remote on a branch; open a PR with the instructor as reviewer; respond to one round of feedback; merge. Your first real code-review experience.

Goal: ship a merged PR. Save the transcript (PR URL, review comments, your responses, merge commit hash) as lab-10-transcript.md.

Estimated time: 90 minutes (including the wait for instructor review; the instructor responds within 24 hours for FND-102).

Prerequisites: Week 10 lecture. Lab 9 complete. A GitHub or GitLab account.

Setup

You need a remote for your ~/fnd-102 repo. Three options:

  • GitHub (https://github.com): free; public or private; the most common. Create a new empty repo named fnd-102 (or fnd102-{your-name}).
  • GitLab (https://gitlab.com for the SaaS, or your academy GitLab at https://git.sandhillscto.com if you have an account): free; public or private. Create an empty project.
  • A self-hosted Git if you have one.

This guide assumes GitHub (the most common); GitLab is structurally identical, with "merge request" instead of "pull request."

Create the remote repo

On GitHub:

  1. Go to https://github.com/new.
  2. Repository name: fnd-102.
  3. Private (visible only to you and reviewers).
  4. Do NOT add a README, .gitignore, or license; you have an existing local repo.
  5. Click Create.
  6. Copy the SSH or HTTPS URL.

Back in your terminal:

cd ~/fnd-102
git remote add origin git@github.com:YOUR-USERNAME/fnd-102.git
# OR (HTTPS form, requires a personal access token for push):
# git remote add origin https://github.com/YOUR-USERNAME/fnd-102.git

git push -u origin main

If SSH push fails with "Permission denied (publickey)," set up your SSH key per https://docs.github.com/en/authentication/connecting-to-github-with-ssh.

Part A: Create a feature branch (10 min)

cd ~/fnd-102
git switch -c feature/disk-usage-improvements

The branch name follows convention: feature/<short-description>.

Make a small but meaningful change to your lab-9/lab-9-du.py. Suggestions:

  • Add a --exclude PATTERN argparse argument that filters out matching directories
  • Add a --min-size N argument that only shows directories above N kilobytes
  • Refactor the parsing into a separate function with a docstring

Pick one. The point is to have a real, reviewable change.

Commit:

git add lab-9/lab-9-du.py
git commit -m "Add --min-size flag to disk-usage reporter

Filters out directories smaller than the threshold (kilobytes).
Useful for large trees where the long tail of small directories
crowds the top-N summary."

The commit message follows the conventional shape: imperative voice; ~50-character first line; blank line; multi-paragraph body explaining the why.

Part B: Push the branch (5 min)

git push -u origin feature/disk-usage-improvements

The output prints a link to open a PR:

remote: Create a pull request for 'feature/disk-usage-improvements' on GitHub by visiting:
remote:      https://github.com/YOUR-USERNAME/fnd-102/pull/new/feature/disk-usage-improvements

Click it (or paste into your browser).

Part C: Open the PR (10 min)

The PR form:

  • Title: "Add --min-size flag to disk-usage reporter"
  • Description: copy this template and fill in:
## Summary

This PR adds a `--min-size N` flag to `lab-9/lab-9-du.py`. The flag filters
out directories smaller than N kilobytes from the report, which is useful
for large trees where the long tail of small directories crowds the top-N
summary.

## What changed

- `lab-9/lab-9-du.py`: added `--min-size` argparse argument; filters the
  entries list after parsing `du` output

## Why

Without this flag, running the tool on `/usr` produces ~3000 entries; the
top-N summary buries the largest entries below the cumulative-1KB-each
tail. With `--min-size 1024` (1 MB), only meaningful directories appear.

## How to test

```bash
python3 lab-9/lab-9-du.py /usr/share --top 10           # without filter
python3 lab-9/lab-9-du.py /usr/share --top 10 --min-size 1024   # filtered

Reviewer asks

  • Does the --min-size value need a unit (KB vs MB)? Currently it's documented as kilobytes; happy to change to a parsed-unit string (e.g., '1M', '500K') if that reads better.
Click "Create pull request." Note the PR number (`#1` for your first one).

Email the instructor or post in the academy channel: "FND-102 Lab 10 PR ready: `<URL>`." The instructor reviews within 24 hours.

---

## Part D: Respond to feedback (30 min after review arrives)

The instructor leaves at least one comment. Common comments:

- "Help string for `--min-size` says 'in kilobytes' but the variable is named `min_kb`; consider naming `--min-size-kb`."
- "Where is the test for this? At minimum verify that `--min-size 0` is a no-op."
- "Consider validating that the value is non-negative; right now `--min-size -1` is accepted."
- "Commit message is good, but the PR description should also note this is from Lab 10."

For each comment:

1. Decide: agree (most common; just make the change) or disagree with reason (rare; ask before pushing back).
2. Make the change in your local branch:
   ```bash
   # edit the file
   git add lab-9/lab-9-du.py
   git commit -m "Validate --min-size is non-negative"
   git push
  1. The PR updates automatically. Mark the comment "Resolved" in the UI.
  2. Reply on the PR thread with a one-sentence acknowledgement ("Done; added a parser.error() for negative values").

The back-and-forth may take 2-3 rounds for the lab. Real PRs sometimes take days; the lab compresses the timeline.

Part E: Merge (10 min after final approval)

When the instructor leaves an approval, click "Squash and merge" (preferred) or "Merge pull request." Confirm the squash commit message. Click "Confirm squash and merge."

After merge:

  1. Delete the branch (the UI offers a button).
  2. Pull the merged change back to your local main:
    git switch main
git pull
git branch -d feature/disk-usage-improvements
  3. Note the merge commit hash: git log -1 --oneline.

Part F: Record the transcript (15 min)

Create ~/fnd-102/lab-10/lab-10-transcript.md:

# Lab 10 Transcript

## PR

- URL: https://github.com/YOUR-USERNAME/fnd-102/pull/1
- Title: Add --min-size flag to disk-usage reporter
- Branch: feature/disk-usage-improvements -> main
- Merge commit: <hash from git log -1>
- Total commits on branch: <count>
- Total review rounds: <count>

## Reviewer comments and my responses

### Round 1

> [Reviewer comment]: Help string says 'in kilobytes' but variable is named `min_kb`; consider naming --min-size-kb.

[My response]: Renamed the flag to --min-size-kb; updated help string.

### Round 2

> [Reviewer comment]: Consider validating that the value is non-negative.

[My response]: Added `parser.error()` for negative values. Tested with `--min-size-kb -1`; now errors cleanly.

## What I learned

- The first PR-comment cycle felt longer than I expected.
- Naming consistency (flag vs internal variable) is the kind of detail that
  reviewers catch and authors miss.
- The squash-and-merge produced a single commit on main; the 3 branch commits
  are preserved in the PR but not in main's history.

## What I would do differently

- Write the help string carefully on the first commit, not the third.
- Add the validation up front rather than waiting for review to catch it.

Commit:

cd ~/fnd-102
git add lab-10/lab-10-transcript.md
git commit -m "Add Lab 10 transcript with PR URL and review reflection"
git push

This is a direct-to-main commit (no PR for the transcript itself; the transcript IS the lab artifact). Some teams require ALL changes to go through PRs; for FND-102 the artifact-only commits go direct.

Expected output / artifact

  • A merged PR in your remote with at least 2 commits on the branch (initial change + at least one response to review)
  • lab-10-transcript.md in ~/fnd-102/lab-10/ with the PR URL, merge hash, review-round count, and a brief reflection
  • Your local main is up-to-date with the merged change
  • The feature branch is deleted both locally and on the remote

What's the failure mode?

This workflow's likely failure modes:

  1. Push permission denied. Your SSH key is not registered with GitHub/GitLab, OR you are pushing with HTTPS but do not have a personal access token. Set up SSH (preferred) or generate a token.
  2. The PR has no reviewer. GitHub does not automatically assign one; you must add the instructor as a reviewer or send them the link.
  3. Branch falls behind main during review. If a different PR is merged into main while yours is open, your branch may need to merge or rebase main. git pull --rebase origin main from your feature branch handles this.
  4. You squash-merged but forgot to delete the branch. Cosmetic, not functional. Use the UI button.

Common pitfalls

  • Committing directly to main by accident. Always git switch -c feature/X first; never commit to main directly. Some teams enforce this via branch protection rules.
  • Force-pushing to a shared branch. git push --force can rewrite history; if a reviewer was looking at the old commits, their links break. Use --force-with-lease if you must force-push.
  • Cryptic commit messages. "wip" "fix" "asdf" are noise. Each commit message should be a sentence. The discipline is the discipline; do not skip it for "small" changes.
  • PR description left blank. The PR description is your reviewer's first impression. A blank description means the reviewer reads the code without context; they will catch fewer bugs because they do not know what to look for.

Stretch (optional)

  1. Open a second PR. Make another small change to a different file; open a PR; merge yourself (no reviewer this time). The point is to feel the workflow without the social cost of the review wait.
  2. Use the gh CLI. Install GitHub's CLI from https://cli.github.com/. Try: gh pr create --fill, gh pr view 1, gh pr review 1 --approve. The CLI is faster than the web UI once you know it.
  3. Add a .github/pull_request_template.md (or GitLab equivalent) so future PRs auto-populate the description with the five-section template from Part C.
  4. Branch-protect main. In repo settings, require PR review before merging to main. Now you cannot accidentally commit to main directly. Future labs all use PRs.

Lab 10 v0.1.