Almost impossible to pinpoint what changed thanks to thousands of lines of completely irrelevant changes and shitty commit messages. It seems the only changeset that might be relevant out of +2,273 -1,531 is the +11 -7 from https://github.com/basujindal/stable-diffusion/pull/103/comm...? Does it even work?
I would say large contributions from non-members generally work rather terribly in open source. If you absolutely have to,
- 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.
This applies just as much if not more in a closed team setting.
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.
> 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.
I'm exaggerating a bit of course, but the point still stands. Git diff and git status even with color are practically impossible to read and understand past a couple changes, and if it's across multiple files and code being re-arranged it's hell. And getting a junior to wire up a GUI diff tool to display and show those items? Impossible. Also, getting juniors to commit parts of a file or only a subset of them? Again, impossible.
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.
I had good success with this flow for major changes (both in-house as well as contributing to FOSS). The target was clear & discussed upfront (sans details that pop up during the actual work), and achieved by a series of consecutive PRs. It's important to be constructive, communicate the intent properly and give the other party enough time to digest the info and think properly about it. Obviously this also depends on the maintainers and their mentality.
in this case a lot of the large change is actually "upstream".
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")
Branching in git is very cheap, so the typical path a large change takes to the main branch is by a series of smaller, incremental PRs. This keeps your work close to the main branch as much as possible.
