First off, I want to say thanks for the effort you're putting into crawl4ai. You clearly care about the project and you're picking real issues that matter. Two of your PRs got merged today (#1829, #1824), and the fixes in them are solid. I want to share some feedback that'll help us work together more smoothly going forward.
You pick real problems. MCP SSE crash, screenshot distortion, deep crawl timeouts, a CVSS 10.0 Redis CVE — these are all things users actually hit. That matters a lot.
Your fixes are technically sound. window.stop() before session reuse, CSS dimension freezing for screenshots, raw ASGI Route for the MCP handler — all correct approaches. You understand the codebase.
Good test coverage. Every PR comes with tests and your descriptions are thorough and well-structured. That's better than what I see from most contributors.
This is the main thing. PR #1828 had a legitimate 2-line MCP fix — but it was buried in 27,000 lines across 151 files. New modules (antibot_detector.py, remove_consent_popups.js, cloud/cli.py), massive refactors to core files (browser_manager.py +867 lines, extraction_strategy.py +743 lines), doc changes, config changes — none of that was related to the SSE crash.
I can't safely merge a PR that large without reviewing every line. The actual fix was great, but I had to extract it myself and apply it separately because the PR was unshippable as-is.
Same thing with #1830 — a valid 11-line Dockerfile fix bundled with 4 files from already-merged #1829. And #1826 bundles 3 unrelated fixes together.
The rule is simple: one issue = one PR. If your commit message needs "and" in it, it should be two PRs.
Today you opened #1831, then #1832 replaced it 13 minutes later. #1826 and #1833 both fix #1484. #1830 re-included files from #1829 after it was already merged. This creates a lot of noise on my end — I have to figure out which of several competing PRs is the "real" one.
Better approach: one branch per issue, iterate on that branch, don't open competing PRs.
#1828 included changes to IMPORTANT_ATTRS (class/id attributes) that were already merged via #1782. The CLI encoding was partially done via #1789. A quick git log develop -50 before submitting would catch these.
11 PRs in one day is a lot. I'd rather see 2-3 clean, focused PRs than 11 that need triaging. Submit a couple, wait for my feedback, then send more. That way you're not duplicating work or stepping on things that are already handled.
-
One issue per PR, always. Fix #1484? That's one PR with just the content_scraping_strategy change. Fix #1254? Separate PR.
-
Rebase on develop first.
git fetch origin && git diff origin/develop -- <your files>before pushing. Make sure your changes aren't already there. -
Clean diffs only. Run
git diff --stat origin/developbefore opening the PR. If there are files you didn't intend to change, remove them. Don't include tooling config (.claude/dirs,PR-TODOLIST.md, etc.) in your submissions. -
Submit in small batches. 2-3 PRs → wait for review → iterate → submit more.
-
Start fresh branches from current develop. After a PR is merged or closed, don't carry its files into the next one.
You're technically capable — that's clear from the fixes that got merged today. The code quality is there. It's really just about PR hygiene and workflow. Tight scope, clean diffs, check the current state of develop first. Do that and your PRs will get merged faster with less back-and-forth.
Looking forward to your next contributions. Keep at it.
— unclecode