Bài 25: Pull Request đầu tiên cho project lớn — quy trình và etiquette

Sau khi hoàn thành bài này, bạn sẽ:

• Hiểu sự khác biệt giữa PR vào repo của mình và PR vào project OSS lớn
• Nắm checklist cần hoàn thành trước khi open PR
• Thực hiện được 12 bước từ fork đến PR merged
• Biết viết PR description đúng chuẩn
• Xử lý được CI fail và review feedback đúng cách
• Tránh các pitfall phổ biến làm PR bị reject

Khi bạn mở PR vào repo của mình, không ai judge bạn nếu commit message lộn xộn hay test thiếu. Khi bạn mở PR vào project lớn, maintainer đang đọc code của một người hoàn toàn xa lạ — và họ phải quyết định có nên merge hay không trong khi còn rất nhiều PR khác đang chờ.

First impression với maintainer

Maintainer của dự án lớn (Hugging Face Transformers, scikit-learn, LangChain...) nhận hàng chục PR mỗi tuần. PR của bạn cần đủ rõ để họ hiểu mục đích trong 30 giây đầu đọc title và description. PR thiếu context, test fail, hay không reference issue sẽ bị bỏ qua trước.

Reputation trong community

GitHub là public. Maintainer nhớ contributor tốt và contributor gây phiền. Một PR chăm chỉ, clean code, respond nhanh review → lần sau PR của bạn được ưu tiên review. Ngược lại, PR abandon giữa chừng, hoặc react defensive với feedback → khó merge lần sau.

Mistake nhỏ → reject

Một số project có standard rất cao. Nếu bạn mở PR mà:

• Không có issue được approve trước
• CI fail mà không fix
• Thay đổi không theo CONTRIBUTING.md

...PR sẽ bị đóng, có khi không kèm comment giải thích. Lần sau bạn mở PR trong cùng repo, maintainer nhìn profile và nhớ.

Hoàn thành toàn bộ checklist này trước khi click "Create Pull Request":

- [ ] Đã đọc CONTRIBUTING.md (toàn bộ, không skim)
- [ ] Đã đọc CODE_OF_CONDUCT.md
- [ ] Issue đã tồn tại (hoặc bạn tự tạo và được chấp nhận)
- [ ] Có comment "I'd like to work on this" được maintainer approve
- [ ] Fork + sync với upstream main (không phải fork cũ tháng trước)
- [ ] Dev env setup đúng theo CONTRIBUTING (không phải pip install random)
- [ ] Test pass local toàn bộ (pytest / npm test / cargo test...)
- [ ] Linter / formatter chạy clean (không có warning)
- [ ] Đã đọc 2-3 PR merged gần đây để học pattern commit message, PR title
- [ ] PR size hợp lý (dưới 300 dòng thay đổi là tốt cho PR đầu)

Tại sao phải có issue trước

Bước "có issue được approve" là quan trọng nhất và hay bị bỏ qua nhất. Bạn có thể implement feature hoàn chỉnh, test đủ, code sạch — nhưng nếu maintainer không muốn feature đó, PR vẫn bị đóng. Hỏi trước, code sau.

Nếu repo chưa có issue liên quan, tạo issue mô tả bug/feature, đợi maintainer phản hồi. Khi maintainer comment "go ahead" hoặc assign issue cho bạn, lúc đó mới bắt đầu code.

Bước 1 — Fork repo

gh repo fork OWNER/REPO --clone
cd REPO
git remote add upstream https://github.com/OWNER/REPO.git
git remote -v
# origin    https://github.com/YOUR_USERNAME/REPO.git (fetch)
# upstream  https://github.com/OWNER/REPO.git (fetch)

Fork tạo bản copy repo về account của bạn. upstream là remote trỏ về repo gốc — bạn sẽ cần nó để sync.

Bước 2 — Sync với upstream

git fetch upstream
git checkout main
git merge upstream/main
git push origin main

Luôn sync trước khi tạo branch. Fork cũ 1 tuần có thể đã bị upstream vượt hàng chục commit, dẫn đến conflict khi merge PR.

Bước 3 — Tạo feature branch

git checkout -b fix/empty-input-handling

Đặt tên branch theo convention của repo (đọc CONTRIBUTING). Phổ biến nhất: fix/mô-tả-ngắn, feat/tên-feature, docs/cập-nhật. Không commit thẳng lên main của fork.

Bước 4 — Setup dev environment

python -m venv .venv
source .venv/bin/activate       # macOS / Linux
# .venv\Scripts\activate         # Windows

pip install -e ".[dev,test]"    # editable install với extras
# Hoặc theo CONTRIBUTING của repo (có thể dùng hatch, poetry, uv...)

Đừng dùng global Python environment. Nhiều CI check version cụ thể — nếu local bạn dùng Python 3.12 mà CI chạy 3.10, type annotation mới sẽ fail CI.

Bước 5 — Reproduce issue

Trước khi sửa, viết test case để reproduce lỗi. Test này sẽ fail — đó là điểm xuất phát.

# tests/test_foo.py
def test_foo_empty_input():
    """Reproduces #1234: foo() raises ValueError on empty list."""
    result = foo([])
    assert result is None  # documented behavior

Chạy để confirm fail: pytest tests/test_foo.py::test_foo_empty_input -v

Bước 6 — Implement fix

Nguyên tắc: smallest change để fix. Tránh refactor code không liên quan trong cùng PR. Đọc 1-2 file lân cận để học code style trước khi viết — indent, naming convention, docstring format.

# src/foo.py
def foo(items: list) -> int | None:
    """Process items and return result.

    Returns None if items is empty (see issue #1234).
    """
    if not items:
        return None
    # ... existing logic ...

Bước 7 — Add test và chạy full suite

pytest tests/ -v
# Đảm bảo:
# - Test mới pass
# - Tất cả test cũ vẫn pass (no regression)
# - Coverage không giảm (nếu repo check coverage)

Bước 8 — Lint và format

# Chạy theo CONTRIBUTING — mỗi repo khác nhau
ruff check . --fix
black .
mypy src/

# Hoặc có thể là:
# flake8 src/ tests/
# isort .
# pylint src/

Nếu CONTRIBUTING không nói rõ, xem .pre-commit-config.yaml — đó là source of truth cho linter/formatter của repo.

Bước 9 — Commit theo convention

git add src/foo.py tests/test_foo.py
git commit -m "fix: handle empty input in foo() (#1234)"

Nhiều project dùng Conventional Commits (fix:, feat:, docs:, test:...). Đọc commit history của repo để xác nhận format. Reference issue number trong commit message.

Bước 10 — Push lên fork

git push -u origin fix/empty-input-handling

Bước 11 — Open PR trên GitHub

Sau khi push, GitHub thường hiện banner "Compare & pull request". Hoặc dùng CLI:

gh pr create \
  --title "fix: handle empty input in foo() (#1234)" \
  --body "$(cat pr-body.md)" \
  --base main

Lưu ý khi điền thông tin PR:

• Title: ngắn, descriptive, reference issue
• Description: dùng template nếu repo có (xem section 5)
• Linked issue: ghi "Closes #1234" để issue tự đóng khi merge
• Label: theo hướng dẫn CONTRIBUTING (bug, documentation...)
• Reviewer: không tự assign, trừ khi CONTRIBUTING nói rõ
• Draft PR: nếu chưa xong, mở dưới dạng Draft để thông báo bạn đang làm

Bước 12 — Đợi CI và review

Sau khi mở PR, CI pipeline sẽ chạy (GitHub Actions, CircleCI...). Theo dõi kết quả:

• CI xanh → đợi maintainer review (thường vài ngày đến vài tuần)
• CI đỏ → debug và fix ngay trước khi maintainer nhìn vào (xem section 6)

Nếu repo không có template sẵn, dùng cấu trúc sau:

## Summary

Fixes #1234 — function `foo()` raised `ValueError` on empty list input
instead of returning `None` as documented.

## Changes

- Add early return in `foo()` for empty input
- Add test case `test_foo_empty_input`

## Testing

- [x] All existing tests pass (`pytest tests/ -v`)
- [x] New test added covers the bug
- [x] Manually tested on Python 3.11

## Notes for reviewer

I followed the pattern in `bar()` which already handles empty input.
If you prefer a different approach (e.g., raise a custom exception),
happy to update.

---

Closes #1234

Tại sao cần "Notes for reviewer"

Section này cho maintainer biết bạn đã suy nghĩ về tradeoff, không phải chỉ code xong submit. Nếu bạn có chọn approach A thay vì B, giải thích ngắn gọn. Điều này giảm vòng lặp review vì maintainer hiểu reasoning của bạn trước khi đặt câu hỏi.

Những gì không nên viết trong description

• "I hope this is correct" — thể hiện bạn không tự test
• Liệt kê lại toàn bộ diff — maintainer đọc diff trực tiếp
• Description rỗng hoặc chỉ "Fix bug" — không đủ context
• Xin lỗi quá mức về code quality — fix vấn đề trước khi mở PR

PR có CI đỏ mà không fix là dấu hiệu rõ ràng bạn chưa test kỹ. Maintainer sẽ không review PR có CI fail. Fix CI trước, mọi thứ sau.

Test fail

Click vào CI job trên GitHub → xem log output → tìm dòng FAILED cụ thể. Thường là:

• Test dùng Python version khác (CI chạy 3.10, local bạn chạy 3.12)
• Test phụ thuộc vào thứ tự chạy (test isolation vấn đề)
• Import path sai sau khi bạn refactor

# Reproduce CI environment local
python3.10 -m pytest tests/ -v
# Hoặc dùng tox để test multi-version:
tox -e py310

Lint fail

# Chạy đúng linter CI dùng (đọc CI config)
ruff check . --output-format=github
# Fix tự động nếu có thể:
ruff check . --fix

Type check fail

mypy src/ --strict

Thêm type hint đúng thay vì dùng # type: ignore — comment này chỉ chấp nhận khi thư viện third-party không có stub.

Coverage drop

Nếu CI check coverage và báo giảm, thêm test case. Không dùng # pragma: no cover tùy tiện.

CLA check fail

Một số project (TensorFlow, scikit-learn) yêu cầu ký Contributor License Agreement (CLA). CI sẽ fail với comment hướng dẫn ký. Làm theo hướng dẫn — thường là comment "I have read the CLA Document and I hereby sign the CLA" trong PR.

Sau khi fix CI

git add .
git commit -m "fix: resolve lint and type errors"
git push origin fix/empty-input-handling
# CI sẽ chạy lại tự động

Maintainer hỏi clarification

Reply rõ, không defensive. Nếu câu hỏi chưa rõ, hỏi lại:

Thanks for the question. Could you clarify whether you mean
approach A or approach B? I want to make sure I implement
what you have in mind before pushing more changes.

Request changes

Implement → push → reply tóm tắt những gì đã thay đổi:

Updated in commit abc1234:
- Changed foo() to raise ValueError instead of returning None
- Added test for the ValueError case
- Updated docstring accordingly

Không chỉ reply "Done" mà không nói cụ thể — maintainer có nhiều PR đang review, họ cần biết commit nào thay đổi gì.

Disagree với feedback

Disagree là bình thường. Giải thích reasoning + cite evidence (doc, PEP, paper):

I understand your concern about performance. Based on my benchmarks
with 10k items, approach A is ~2x faster than approach B on Python 3.11
(see attached gist). Happy to discuss further or defer to your judgment.

Không argue dài dòng. Nếu maintainer vẫn giữ quan điểm sau khi bạn đã giải thích, implement theo họ.

Approve và merge

Thank you for the review and for merging! Looking forward to
contributing more to this project.

Ngắn gọn. Không cần cảm ơn quá nhiều lần.

PR bị reject (closed without merge)

• Không take personally — maintainer có thể có lý do kỹ thuật hoặc roadmap mình không biết
• Cảm ơn thời gian review nếu họ có giải thích
• Rút kinh nghiệm: scope quá rộng? không có issue trước? không đúng direction?
• Move on — tìm issue khác hoặc repo khác

Một số feedback hay gặp và cách xử lý:

"Can you add a test?"

Add test cho case bị chỉ ra, push. Không hỏi lại "test thế nào?" — nhìn vào file test hiện có của repo để học pattern.

"This is out of scope"

Tách phần extra ra thành PR riêng. Commit gốc giữ lại, tạo branch mới từ main cho thay đổi phụ:

git checkout main
git checkout -b feat/separate-feature
# cherry-pick chỉ commit liên quan
git cherry-pick <commit-hash>
git push origin feat/separate-feature

"Please follow our style"

Đọc style guide (link thường trong CONTRIBUTING). Chạy formatter. Nếu chưa rõ cụ thể cần sửa chỗ nào, hỏi: "Could you point to a specific example in the codebase I should follow?"

"This breaks API compatibility"

Cần tìm cách backward-compatible: thêm parameter mới với default value thay vì thay đổi signature. Hoặc deprecate path cũ trước khi xóa.

"We don't need this feature"

Đây là judgment call của maintainer. Không tranh luận dài. Nếu bạn nghĩ feature có giá trị, hỏi "Is there a way to implement this that would fit the project's direction?" — một lần, không lặp lại.

Khi nào force push là OK

Rebase PR branch lên upstream/main mới — bạn cần force push sau đó. Đây là thao tác bình thường:

git fetch upstream
git checkout fix/empty-input-handling
git rebase upstream/main
# Resolve conflict nếu có
git push --force-with-lease origin fix/empty-input-handling

--force-with-lease an toàn hơn --force: nó fail nếu có commit trên remote mà local bạn chưa có — ngăn chặn việc vô tình xóa commit của người khác.

Khi nào tránh force push

Tránh force push ngay sau khi maintainer đã review và để lại comment. Amend hoặc rebase lại sẽ làm commit hash thay đổi — reviewer mất context về comment cũ. Pattern an toàn hơn: append commit mới cho mỗi round review, squash chỉ khi maintainer yêu cầu hoặc khi sắp merge.

Squash commits

# Squash 3 commit cuối thành 1
git rebase -i HEAD~3
# Trong editor: đổi "pick" thành "squash" cho commit 2 và 3

Chỉ squash khi maintainer yêu cầu hoặc project dùng "Squash and merge" strategy (check repository Settings → General → Merge button).

PR size	Lines changed	Nhận xét
Beginner / first PR	< 100	Lý tưởng. Review nhanh, ít risk
Standard	100 – 500	Chấp nhận được nếu scope rõ ràng
Large	500 – 1000	Xem xét split nếu được
Very large	> 1000	Thời gian review rất dài, risk reject cao hơn

PR lớn không đồng nghĩa với PR tốt. Maintainer không có thời gian review 2000 dòng diff cẩn thận. Split thành nhiều PR nhỏ, mỗi PR một mục đích rõ — review nhanh hơn, feedback dễ hiểu hơn, merge sớm hơn.

Ngôn ngữ và tone trong PR

• Dùng tiếng Anh — hầu hết OSS project dùng English là ngôn ngữ mặc định
• Khiêm tốn: "I think...", "Maybe...", "Could you..."
• Tránh: "Obviously", "Clearly", "Just do X" — nghe rude dù không có ý
• Nếu không tự tin tiếng Anh: nhờ Claude hoặc Grammarly check description trước khi submit

Maintainer là volunteer. Không có SLA. Một số project nhỏ chỉ có 1-2 maintainer và họ có việc riêng. Đây là timeline thực tế:

• Sau 1 tuần không có phản hồi: ping lịch sự trong PR comment. Một dòng, ví dụ: "Hi, just checking in — happy to update if there's anything needed before review."
• Sau 2 tuần: hỏi trên Discord / Slack / Discussions của repo nếu có.
• Sau 1 tháng: move on. Có thể giữ PR mở (không đóng) và tiếp tục contribute repo khác.

Không ping mỗi ngày. Không thay thế maintainer bằng cách assign reviewer ngẫu nhiên. Không complain public về thời gian review.

Time investment thực tế cho PR đầu tiên

• Tìm issue phù hợp: 30 phút – 2h
• Đọc CONTRIBUTING + học codebase: 1–3h
• Implementation: 1–5h
• Test + lint + fix CI: 30 phút – 2h
• Viết PR description: 15–30 phút
• Review cycle (đợi + respond): vài ngày – 2 tuần

Total realistic cho first PR merged: 5–15h effort, 1–4 tuần wall-clock time.

Những sai lầm hay gặp nhất:

Mở PR không có issue

Maintainer nhìn vào PR không hiểu "why" — tại sao cần thay đổi này? Luôn link issue hoặc giải thích context trong description.

Quên "Closes #N" trong description

Issue sẽ không tự đóng khi PR merge. Maintainer phải đóng thủ công. Nhỏ nhưng hay quên.

PR refactor architecture chưa thảo luận

Refactor lớn phải discuss trong issue trước. Code xong rồi mới hỏi → maintainer khó accept vì không biết hướng đi của bạn có align với roadmap không.

PR fix style toàn repo

"Tôi sẽ reformat toàn bộ file để đúng PEP 8" → PR có 500 file changed → reviewer không thể nhìn ra diff thực sự → reject. Chỉ format file bạn đang sửa.

Mix nhiều fix vào 1 PR

Fix bug A + add feature B + refactor C trong cùng PR → harder to review, harder to revert nếu có vấn đề. Mỗi PR một mục đích.

CI fail, không fix, push thêm commit khác

Maintainer sẽ thấy PR có CI đỏ và tiếp tục đỏ sau nhiều commit — signal bạn không chú ý.

Force push sau khi reviewer đã để comment

Commit hash thay đổi → reviewer mất thread comment. Append commit mới, không amend.

Quên ký CLA

CI fail với message về CLA → đọc hướng dẫn và ký trước khi expect review.

Test pass local, fail CI platform khác

Windows path separator, case-insensitive filesystem, timezone... Reproduce CI environment local hoặc dùng GitHub Codespaces.

Drama trong PR comment

Bất đồng kỹ thuật thì argue kỹ thuật — cite doc, benchmark, PEP. Không attack cá nhân, không passive aggressive. Code review là technical discussion, không phải personal evaluation.

Dọn dẹp branch

# Xóa branch remote (GitHub thường tự offer khi merge)
git push origin --delete fix/empty-input-handling

# Xóa branch local
git checkout main
git branch -d fix/empty-input-handling

# Sync local với upstream
git fetch upstream
git merge upstream/main
git push origin main

Document cho CV và LinkedIn

Mỗi PR merged: bookmark URL, ghi lại project + summary thay đổi + impact (nếu có số cụ thể).

LinkedIn post ngắn nếu muốn chia sẻ:

Just got my first PR merged into [project] (N GitHub stars).
The change fixes [summary — 1 câu]. Learned: [1-2 lesson cụ thể].
Thanks to [maintainer handle] for the thorough review.

Concise. Không humble brag. Tập trung vào what you learned, không phải "tôi giỏi như nào".

GitHub profile và README

Update Profile README: thêm link đến PR merged. Update LinkedIn About: "Contributor to [project]". Khi CV đủ PR, có thể thêm mục "Open Source Contributions" với URL cụ thể.

Long-term journey

Steady 1–2 PR mỗi tháng vào cùng repo → maintainer nhận ra tên bạn → triage issue được assign cho bạn → dần được invite vào team. OSS reputation tích lũy chậm nhưng bền hơn so với listing "familiar with X framework" trên CV.

Bài 26: Cấu trúc CV cho AI Engineer trái ngành — 1 trang đủ chưa?