leejet
Allow @stduhpf and @Green-Sky to approve PRs
@stduhpf is a prolific coder with many PRs added to this project. It would be helpful to provide @stduhpf with write access to this repository so that development of stable-diffusion.cpp can continue when @leejet is unavailable.
If direct write access is not desired, we can do some review requirement from a review team. Like @stduhpf and myself.
@Green-Sky I think a review team is a better idea. Since I'm more active than @leejet right now, giving me direct write acess would kinda feel like taking over, and I don't want that.
Yes. llama.cpp does that too. I claim that was one of the reasons why it took off and scaled so well in the early days.
In the settings of the Github repo, you can create a rule that requires a number of approvals to merge a PR. For example:
This is what we use in the llama.cpp repo and it works OK IMO.
Thanks for your feedback, everyone. I've updated the issue title to reflect the discussion.
Perhaps we could have a 'staging' branch too, for more involved changes? That way, additional functionality could be pre-merged and better tested without disrupting the releases. There is a lot of relatively simple pending PRs that could probably be integrated just by exposing them to more testing.
I could also help with reviewing and testing PRs (as long as that doesn't count as approval; I'm not that familiar with the codebase).
Copying the comments from #696 for easy reference:
@kgigitdev:
I'm sure that nobody would take offence if a temporary (friendly, and prominently attributed) fork were created to contain suitably-approved merged and conflict-resolved branches from all the developers who have pending PRs, as well as lots of useful work that is currently being duplicated across lots of forks.
@Green-Sky:
If we ended up doing this, I would ask @ggerganov to host that project at the ggml org. But let's wait a while longer and see what @leejet ends up doing.
Hopefully optional PRs remain optional, and the library does not get bloated. Though current open PRs seem to be alright.
It's been over three months since @leejet last contributed according to Github's tracker, a much longer period than previous hiatuses. Is it time to migrate to the bleeding edge code maintained by @stduhpf?
It's been over three months since @leejet last contributed according to Github's tracker, a much longer period than previous hiatuses. Is it time to migrate to the bleeding edge code maintained by @stduhpf?
I think that is already naturally happening. But don't treat https://github.com/stduhpf/stable-diffusion.cpp/tree/bleedingedge as stable 😄
I think that is already naturally happening.
At the moment issues and PRs are targeted at this repo, rather than downstream. Yes, many PRs are from @stduhpf, but some are not (e.g. #646, #658, #662, #680, #702, etc.).
But don't treat https://github.com/stduhpf/stable-diffusion.cpp/tree/bleedingedge as stable 😄
If that repo is promoted to main, I reckon that an important part of the transition would be to make it more stable, perhaps prior to moving it to the ggml-org.
There is "stable", and there is "bugs are actually getting fixed there" 🙂
As much as I want the new features myself, IMHO it's best to set up the new process with simpler stuff first. We could start by promoting a branch of one of the forks to a "next-stable-release", and merge pending fixes, ggml updates and related changes, maybe simpler stuff (like leejet/stable-diffusion.cpp#316 , which is 11 months old 🙁) - only reviewed PRs, no direct changes.
And if that config @ggerganov suggested is per-repo instead of per-branch, I could set it up on my own fork, so we can give it a try.
I've reviewed and tested a few of the smaller PRs that had no reviews / approvals.
These look good to me: #316, #422, #468 , #591, #629 .
These IMHO would need changes to be accepted: #457, #631 .
These are doc changes, I'm not sure what would be the criteria for accepting: #333, #453 .
There is "stable", and there is "bugs are actually getting fixed there" 🙂
The bleeding edge branch is stable enough to be a candidate for use downstream (by Koboldcpp). That suggestion gives me some confidence in its potential as a new-stable main in the absence of @leejet.
We should push this further. As long as leejet is invited and included respectively going forward, I see no issue in focusing a more communal/organizational approach.
It appears that PRs can be migrated to another repository. If that works, then I reckon the current main branch might be the best starting point rather than bleeding-edge.
If we do this, I guess it should be hosted by the ggml org? I could also create a "stable-diffusion.cpp community" repo or whatever and transfer the ownnership later.
Whatever we do, I want @stduhpf to take the lead, since you are currently the most active dev :)
If we do this, I guess it should be hosted by the ggml org?
If @ggerganov is kind enough to agree, then I also reckon that would be best.
I think hosting the project under ggml-org would give it a bit more exposure which would be nice both for the project and for the ggml-org. If we decide to do it, we will assign @stduhpf with admin role and he can take the lead. I think we can also allocate some ggml-ci resources if needed and if it makes sense.
I have a few thoughts that would like to highlight:
- The ggml-org members won't have enough capacity to help with the development and maintenance of the project, at least in the near term. So not much will change in this regard regarding the development.
- I'm quite unfamiliar with the codebase, the quality of the code and generally with the adoption of this project by the community. I don't really expect any serious issues to raise, but just in case, if for whatever reason things somehow take a bad turn and we decide that the project should not be part of ggml-org, we will have the option to remove it from the organization.
- Generally, I am OK with the transfer, but would feel much better if @leejet chimed in with their opinion and plans for the future, although I understand we don't have control over this.
If you decide to give it some more time and create a separate community in the meantime also sounds good to me - we can discuss again later depending on how things develop.
Generally, I am OK with the transfer, but would feel much better if @leejet chimed in with their opinion and plans for the future
We would all prefer to hear from @leejet. However, I agree that it may not be possible. This issue has been open for almost a month, and the last commit from @leejet was in early March.
I think we can also allocate some ggml-ci resources if needed and if it makes sense.
This project would probably need some CI resources for binary compilation. Perhaps we could move to numbered releases, rather than commit-based ones, to minimize the required resources.
~~Currently some important work is going on on the ggml side for implementing conv2d ops that could directly benefit this project. I think we should move to a fork where we will refactor a bit and merge some PRs to catch up with the upstream and prepare for the eventuality of being hosted by ggml-org to make it easier for newcomers. It would help a lot if devs working on llama.cpp could perhaps directly test and experiment with their work on sdcpp~~
Edit: anyway @leejet seems to be back so he better knows than anyone else what's the best move
