- Communicate ahead of time; don't surprise maintainers with sweeping architectural changes or huge features no one wants or would like to review;
- Try to break up changes into logical units that can be understood and reviewed independently;
- Write useful and detailed commit messages (some bad examples: "Update attention.py", "various clean-ups, code now beautified");
- Don't sneak in anything unrelated to the PR; don't sneak anything unrelated into a commit;
- Absolutely don't use a code formatter to format the entire code base if the repo wasn't already using one. You can suggest that separately. And changes like that are best done by a trusted member.
Personally I blame juniors that insist on using the git CLI instead of a million visual GUIs to git. They get told just do a git commit, have no idea what they changed so can't produce a meaningful commit message, and because it's the command line good luck getting a visual representation of what you're committing.
I don't get the point here. I've never met a developer who knew about `git commit` but didn't know about `git diff` (sometimes I pointed out `git diff --cached`).
I don't think you can blame the CLI on this. It's about the lack of proofreading, in my opinion.
Maybe you work with a higher/better caliber of juniors than I do. At this point, I'm seriously contemplating being a mean dictator and forbidding commits outside of a dedicated GUI until they can prove their adeptness at using the CLI, which they should learn on their own time.
and this: https://www.aleksandrhovhannisyan.com/blog/atomic-git-commit...
That way your "large change" becomes a collection of small, easy to reason about and well explained changes.
We Have Neon, who is making a PR.
We have Upstream, SD source.
We have basu, repo maintainer.
Neon pulled in upstream changes, and are trying to merge those into basu's repo AND then on top of that, apply neon edits to the code.
Ways to make this cleaner:
- Basu pulls in upstream changes, Neon just puts theirs on top. (Slow, you have to wait for Basu)
- Neon makes two PRs, one to pull in upstream changes, another for his edits on top of those changes. (More work, and coordination, but each PR is "one unit of work")
- Better commit messages: https://github.com/basujindal/stable-diffusion/pull/103#comm... <- notice the repeated and non-descript commit messages? That makes it hard for non-experts to cherry pick out the bits that are really relevant. (Fastest)