[DISCUSSION] Re-evaluate PR merge policy #8686
Comments
|
cc: @syl20bnr |
robbyoconnor
changed the title
|
Thanks for making this issue. I'm pretty optimistic that merging rights can be shared with long time contributors like @robbyoconnor while still maintaining the final editorial rights to @syl20bnr. A revised policy has the potential to:
|
Indeed, 1000+ issues and hundreds of pull requests increase the friction for any potential patch or issue because it's effort that may just be a waste of time. I imagine it could also scare off some potential new users of the project that might see that number and think that it's not actively maintained. |
|
I discussed a bit with @robbyoconnor and shared my views in the gitter just prior to this issue being created. I thought I would comment here. I'm pretty familiar with spacemacs as a user and reasonably so as a dev - don't know anywhere near as much as current/past maintainers but more than the average spacemacs user. I'm in the gitter an awful lot and help new users quite often. I've made a few small code contributions, had a few more small ones that got stuck. In most cases, in order to get things merged I ended up explicitly pinging @d12frosted. I didn't really feel comfortable with this because it's clearly not scalable and a bit intrusive, but I've had a number of discussions with him in the chat and he encouraged me to ping him if something got too stale. I've also been involved in some suggestions/discussions on larger changes. Some examples: #6763 and #6200 (kind of related). In both cases, there was a decent chunk of work to be done, but I felt like these issues (which both got tagged mailing list) spark some conversation, and then end without resolution. By resolution I mean, that 1 or more maintainers ends up agreeing on a plan of action. Obviously it's not possible to plan every detail and bumps may come up. But without some reasonable communication and plan I think that going off and spending a dozen hours on a PR that may get rejected flat out is not really something I want to do. Another example of areas that get less attention is often language layers. Only a subset of users/maintainers use the language layers so they get less attention (than say helm, ivy, magit, evil, etc), and yet often there's a lot of complexity and choices in the language layers. I dropped the c/c++ language layer because rtags was not getting merged after many many months of efforts. On the other hand it incorporates older tag style solutions (ctags/gtags) that I (and many people I talk to) have no interest in. I wrote my own layer from scratch that uses rtags and ycmd, but I'm not sure whether two different layers for the same language would be tolerated. There's very little incentive for me to upstream it (though I keep linking it for people who are interested in C++ in the gitter). I'd also like to mention that I did actually volunteer to be a maintainer but didn't hear back and was a bit embarrassed to ask about it. I think having maintainers for parts of spacemacs (sub-systems as it were) is a very good idea. There's probably half a dozen people I've seen around that would be interested in being maintainers for certain specific things and could help move things along. I also think (pretty controversial comment coming) that spacemacs should declare "issue/PR bankruptcy". There's so many outstanding issues, and they've lingered so long, we have no way to know if it's still affecting the user, if the contribution is still relevant, etc etc. I think after the maintainer structure has (hopefully) changed a bit and becomes more responsive, we should archive all issues that haven't seen activity for say a month, and encourage people to re-open things they still care about. And then really focus on trying to keep open issues/PR's to a reasonable number. |
|
Piping up as a very new spacemacs user here, the number of open issues and PRs does affect my own inclination towards/against reporting new issues, fearing to add to the clutter. |
|
@ngoonee DO NOT fear adding to the clutter...not reporting bugs is bad...SOMEBODY has to report the bugs...otherwise they'll never fix the bugs! |
|
It's totally understandable that people who spent some time writing a PR or reporting an issue would get upset after being ignored for a long time.. But even with @TheBB and @StreakyCobra working as hard as they could the issue/pr mountain was slowly, but steadily growing, because with every new feature added we get more code that need to be maintained so the situation is bound to get worse with time. But fortunately Spacemacs is a wonderful product(and a quite popular one) that also happens to be free... You know, like Wikipedia and stuff. 🙄 Sooo... how about "Please Read: A Personal Appeal From Spacemacs Founder Sylvain Benner" ? Let people show how badly they need Spacemacs after all 😄 This project can keep on growing only if we have a steady core of experienced maintainers who are incentives to spend a portion of their time on maintaining the project and who are going to be responsible for their work, because they're not just doing us a favor by rolling the stone up the hill - it's also their job. Otherwise Spacemacs will succumb to entropy. Whether it's due to constant breakage or a bunch of unmotivated/bad/inefficient maintainers who require too much @syl20bnr 's time to be maintained themselves |
|
But any kind of improvement to the current situation will require more effort on @syl20bnr 's side. And since he owes us nothing and probably maintains Spacemacs just out of feel of duty... We are kinda screwed here. I wonder, would he start Spacemacs if he knew how much time it will consume? What kind of sacrifices he will need to make 🤔 So the goal number one - make @syl20bnr positively motivated. NOW |
Either I misunderstand or I will have to respectfully disagree. Motivating @syl20bnr to constantly work on spacemacs is unsustainable. Our number one goal should be to find a way to get spacemacs moving forward smoothly without his constant attention.
If this discussion succeeds, then hopefully he will need to sacrifice much much less. At least that is one of the main goals here. |
|
I agree with @rgrinberg . Let's not make drama where none exists; nobody is suggesting that any maintainer neglect anything, or work any harder than they are, or want to. I think the way to look at it is trying to work smarter as a community. If you have PR's that never get reviewed, then somebody's hours are being wasted. Those hours could have been better spent reviewing another PR. So instead of 2 unreviewed PR's that don't get merged or help spacemacs, you end up with 1 PR that does get merged. We need to increase the number of code reviewing and merging man hours available to the spacemacs project. I think everyone agrees that the way to do this is not to ask anyone to spend more of their time, but rather to see if there's a way in which more people can get involved. This obviously requires both willing people, and agreement from @syl20bnr about how this could be done in a way that he is comfortable with. Am confident that this great community and project will find a way forward, cheers. |
|
I'm still relatively new to the project, and emacs in general, but I do agree with @quicknir and @rgrinberg, @syl20bnr has done a ton of work to bring this project to life, and I couldn't imagine not having it. I want to see changes that make his life easier and allow him to focus more on what he wants to work on, not the humongous backlog of PRs. I think this idea extends to all of the maintainers and people putting time in on the project. It is unsustainable to bottleneck on a handful of people, and it is unfair to expect that of them too. I agree with the idea of having area specific maintainers, I don't know that I have enough experience with elisp to be a full maintainer, but I would happily volunteer to jump in more on the languages I work in regularly and love (Ruby, JS, Go). I can review PRs, test, and eventually, if given permission, I'd be happy to start merging working PRs in. However, I think one of the biggest things is that we need the core team to chime in to let us know how we can help, and to discuss options for moving forward and helping them manage their time better |
@jredville It is good idea to make maintainers feel responsibility and ownership over some subset of the Spacemacs code/features. But it is also really easy to break whole Spacemacs by fiddling with a layer in a way that the bug will reveal itself only with certain combination of layers/Emacs version. We need experienced and responsible people and a way to prevent them from burning out in a couple of weeks. Simply adding everyone who volunteer to the collaborators IMHO will make things only worse. @quicknir We can start with code reviews as they don't require extra privileges. How about building a base of volunteers that will be listed in the PR/issue placeholder. Something like "If this is about
I'm not suggesting that we should rely on his work solely. But BDFL will need to take some extra organizing responsibility with more maintainers. Not sure if he's ready or willing to do so (otherwise he would add more people already) However it's probably the only way forward. The question is how to organize and reward those people and @syl20bnr for their work so they will treat it responsibly and won't burn out. |
|
Just an opinion from an inexperienced and irresponsible user.
Something breaks always, even if one contributor adds stuff incrementally. But the language layers are quite independent from the core of Spacemacs. I don't think that it is easy to break the whole Spacemacs by fiddling with them and it is not worth adding external repos/brances. |
|
@dvzubarev I don't think that team/branch thing gonna happen anyway #4890 But then it doesn't look like BDFL wants more people with a write access to the Spacemacs master/dev branches either. also #3591 |
|
I agree that it can be easy to break the whole thing, that's why my perspective was eventually on merges. However, I like the idea of having a base of volunteers that can give better feedback on areas. At the very least I could pull down branches to test and give feedback on. Right now I'm just concerned it would add to the noise |
|
Are there any integration tests for emacs that we could write to ensure correctness? |
While that's an important thing to work on. I think the entire premise that spacemacs will become less stable if more people are allowed to merge is false. As I've already said, I think it will make it much easier to motivate people to test PR's if they were aware that their efforts would count towards merging them. For example, having a concrete rule like 10 people must test a change/fix to a single layer before its merged is likely to increase the quality. Especially given that many PR's that end up getting stalled are simple bug fixes. |
|
you could also have a rule that, moving forward, if you don't have integration tests (not sure how sophisticated the emacs testing can be is though) you don't get to merge it. every feature needs tests. I do agree that the risk of a PR breaking things is minimal if any extra code is properly prefixed with a pseudo namespace that is elisp convention. The only thing that I can see breaking is to have one layer have a conflict with leader bindings or some kind of interaction with some of the shared packages like helm, ivy or company etc. |
Exactly. But note that in those situations: |
|
@fiveNinePlusR @rgrinberg Spacemacs layer system helps with untangling stuff, but Emacs still remains an ancient scary beast that must be respected :) Stuff like evil and autocomplete interact with pretty much everything, there are hooks and advices everywhere, package loading order still meters and some of them can conflict and if a layer is buggy it can make Spacemacs unusable by disrupting the loading process and do it seemingly random. There are numerous occasions when @syl20bnr pushed a harmless refactoring that end up busting Spacemacs for some users. And pretty much every Spacemacs master release (previously tested in develop for a while) requires a bunch of hotfixes. But I don't think this is critical. Commits can be reverted, layers disabled - no one gonna die. The only question is how often this will happen. The more PR's are merged - the more chances are for something to break(multiplied by inexperience of the person merging them). Now all important code changes come directly through @syl20bnr so he knows the code base perfectly, if something breaks he knows where to look. And now Spacemacs is in no way broken. We are getting hotfixes, new stuff slowly but getting merged. I don't know if this strategy is viable in the long run, but I can understand why BDFL not so eager to change it.
Emacs tests can be pretty powerful but 99% of Spacemacs is living 3-d party code that often changes. We need to go smart about writing tests otherwise they will require too much maintenance. Also it's pretty hard to make a fair test given the amount of parts that can interact in the real word. |
|
The very fact that 3rd party packages changes so much is another reason to have regression testing. We need to know when a package breaks things for Spacemacs users. |
You voluntering @CeleritasCelery? Who's gonna update those tests? Who will maintain them? We're trying to reduce, not add to the overhead. |
|
Nobody is against regression testing. It's just that it's completely orthogonal to the issue being discussed here. Regression testing will have a positive effect on the stability of spacemacs. And so will the policy changes discussed here if implemented properly. |
|
@rgrinberg is your change that "will have a positive effect on the stability of spacemacs ... if implemented properly" having 10 people test every PR? and @robbyoconnor if we went the regression testing route, people who submit PR would need to add tests.
I could say the same thing about the whole repo. Who's gonna update Spacemacs? Who will maintain it? Answer: everyone. This is not rocket science. |
|
We have some nice tests for evil in https://github.com/syl20bnr/spacemacs/tree/master/tests Evil is pretty stable packages and a critical one for Spacemacs so it is great that we have them and there probably are some other packages that if covered in a similar way will reduce the effort needed to keep Spacemacs healthy and reduce the urgency of this issue. But yeah. With as big user base as Spacemacs has we can see if something is broken pretty much right away and in the most cases the bug is quickly fixed. So test are good and all but they also need to be constantly synced with upstream, also they can be buggy. And sometimes all this effort can be spent better.... Also we may be able to utilize tests provided with packagers. Gotta find someone who not too disgusted by writing tests :P |
Then we'll have like no PR's and no problems with merging them :) |
And who would fix them when they break? You can't expect the original layer author to be around forever. |
|
I propose to wrap it up and wait for @syl20bnr response. I hope he will not ignore us. Or the next time there will be pitchforks and torches and Spacemacs forks... We need our BDFL to come and reassure his flock about the great future successes. Nothing hurts community as much as a lack of communication |
|
I, too, am sorry to hear about your burnout. that's the absolute worst and it really is debilitating and often not talked about enough. But, I do think this brings up a great point about the issues; namely, I think the issues should be bugs in spacemacs proper and not a place to get general help for spacemacs configuration. Issues should probably be closed quickly if they look like an upstream bug or a user configuration error and push them to gitter.im chat (or maybe a mailing list if that exists) for troubleshooting. (you can always close and say to them that they are welcome to reopen the issue if they think the closing was in error) I do think spreading this load around will prevent the few people working on issues from burnout and being overwhelmed. Good luck with the burnout @TheBB... wishing you all the best! |
|
With so many active users maybe a push for more monthly contributions could go a long way towards allowing syl20bnr to move some time from earning money to working on spacemacs. Currently there's less than 20 people who contribute between $1 and $20 a month. With spacemacs popularity it should be very possible to bring this number to say 200 people giving $10 a month, any maybe even higher. |
|
@mbertheau That will be a lot easier if there was a patreon or something set up with funding goals. We also need to understand the @syl20bnr is doing this as a side project, and it is likely that he wants to keep doing it part time and is enjoying his dayjob etc. How the funding is done for the development of Spacemacs is ultimately a decision by @syl20bnr and how he wants to allocate his time between work and Spacemacs is a personal choice. I am happy about the direction of Spacemacs and exited about what will come either way. I hope to be able to add a montly donation once my school stuff is all done. |
|
@JAremko I'd be happy to be a maintainer of the c-c++ layer. |
|
There is a bountysource. No goals though.
Herman <notifications@github.com> schrieb am Do., 20. Juli 2017, 14:57:
… @mbertheau <https://github.com/mbertheau> That will be a lot easier if
there was a patreon or something set up with funding goals. *We also need
to understand the @syl20bnr <https://github.com/syl20bnr> is doing this as
a side project, and it is likely that he wants to keep doing it part time
and is enjoying his dayjob etc.*
How the funding is done for the development of Spacemacs is ultimately a
decision by @syl20bnr <https://github.com/syl20bnr> and how he wants to
allocate his time between work and Spacemacs is a personal choice. I am
happy about the direction of Spacemacs and exited about what will come
either way. I hope to be able to add a montly donation once my school stuff
is all done.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8686 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIJxyJg2KiIS2DQbL_fptXJlreP8XlXks5sP07MgaJpZM4M6q9_>
.
|
|
I was very happy to find this thread, it's reaffirmed my belief in this project. Thanks to everyone who's worked so hard to bring emacs to diehard vim users like myself 😄. Per @syl20bnr's comment, it sounds like the big organizational changes that will enable spacemacs to move a little faster with the PRs are postponed until 0.300. I'm excited to hear that those changes are on the horizon, but since there is no ETA for 0.300 (indeed, there seems to be no ETA for 0.201 either), I'm wondering what, if anything, people like myself (who love the project and are interested in contributing, but can't dedicate anything more than a few hours a week to it) can do to help carry the burden for the maintainers in the meantime. I guess until 0.300 there's not a whole lot that can be done with the PRs (correct me if I'm wrong), but it sounds like helping respond to issues, especially requests for help troubleshooting, would be productive. Do the core contributors (@bmag @d12frosted @StreakyCobra @justbur) have other suggestions regarding what might be helpful in the short-term? For example, is #9346 something we'd like to see more of? I think @mbertheau's point about donations is a good one. I recognize that @syl20bnr and the other maintainers are most likely perfectly content to keep spacemacs as a side project, but seeing as there's a paypal donation button embedded at the top of the readme as well as a section on supporting spacemacs, I think it's fair to say that there is at least some interest in soliciting donations to help fund development of the project. However, despite using spacemacs for a year and a half, I didn't know there was a donations page until I read this thread. And then when I did find out about it, it took me a couple minutes to locate the bountysource page, which is liked from the bottom of the readme. It's true that I live under a rock with regards to many things, but I use spacemacs every day, so IMO that should not be one of them. I double-checked just now and would like to note that at the time of writing:
This is crazy. IMO this is an amazing project that has benefitted a large number of people, and the maintainers who work so hard to keep it going should not be afraid to ask for donations. I think there should be a "Donate" button in the nav bar of spacemacs.org (or even better, we could stick it next to the "Download" and "Install" buttons), and in the |
|
Hi. Let me start with praise for Spacemacs. It is a fantastic effort that brought emacs out of the last into this millennium (my personal opinion). The above discussion started in April. It is end of October. Is there an updated view on the above mentioned good suggestions how the work on Spacemacs could be organised to handle the large amount of contributions and issues effectivly? |
As discussed in syl20bnr#8686, the PR merge policy could use some streamlining. Here is my humble suggestion on how we could move forward: having an official and structured PR review process that community members can follow. As suggested by @bmag in syl20bnr#8686, this splits the PR process currently wholly borne by collaborators into two phases, review and merge, that can be completed by different people. Paraphasing @JAremko in syl20bnr#8686, in my opinion this - Allows the community to share some of the workload currently born by the maintainers / collaborators - Helps in speeding up merging process because the most of beginner errors are obvious to anyone with a bit of experience and can be fixed right away - Scales efficiently as the project grows - Lowers the barriers to entry to the review process, resulting in a possibly much higher amount of reviewers in the long run - Encourages knowledge sharing - now people are rarely giving advice on code improvements to each other - Encourages active contributors to learn more about best practices - Generates a pool of potential collaborators - a stepping stone that will help to show true skills of a contributor and when someone gets really good and productive at helping to improve PRs you have strong indication that this person is ready to become a collaborator - Helps to reduce the amount of merged bugs (ofc only if the PR reviewer actually tested the code) - Probably has synergy effects with upgraded tooling or tagging contemplated in syl20bnr#8686 - Last but not least, it will get people busy and they will not feel like they are powerless when they want to get something merged sooner What do you think? What should be added to or removed from the review process? Feedback is welcome!
|
I opened a pull request #10055 based on the discussion here, introducing a detailed PR review policy for every community member to follow. This way, community members can become more active participants in the review process and can help to share the work load of collaborators as suggested by @bmag. I thought that this would have great synergies with the upcoming tooling mentioned by @syl20bnr, and would help to speed up merging. Please think of the PR as my humble proposal for a part of the upcoming community portal and tell me what you think of this approach on the PR discussion page :) |
Based on discussion in syl20bnr#8686, introduces a structured PR review process that community members can follow. The PR process currently borne by collaborators is split into two phases, review and merge, that can be completed by different people thus allowing for better scalability. Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with "Pull Request" or "PR" for readability.
Based on discussion in syl20bnr#8686, introduces a structured PR review process that community members can follow. The PR process currently borne by collaborators is split into two phases, review and merge, that can be completed by different people thus allowing for better scalability. Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with "Pull Request" or "PR" for readability.
Add a section in CONTRIBUTING.org mandating a changelog entry in every PR. This is done to share the workload of maintainers and streamline the merge process as suggested by @kain88-de in syl20bnr#8686.
Based on discussion in #8686, introduces a structured PR review process that community members can follow. The PR process currently borne by collaborators is split into two phases, review and merge, that can be completed by different people thus allowing for better scalability. Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with "Pull Request" or "PR" for readability.
Add a section in CONTRIBUTING.org mandating a changelog entry in every PR. This is done to share the workload of maintainers and streamline the merge process as suggested by @kain88-de in syl20bnr#8686.
|
Now that #10055 is merged we have a PR review policy in CONTRIBUTING.org and community members can help a PR get merged by reviewing and testing it. So, if you feel like contributing, you can do some reviews! Happy hacking 😊 |
Based on discussion in syl20bnr#8686, introduces a structured PR review process that community members can follow. The PR process currently borne by collaborators is split into two phases, review and merge, that can be completed by different people thus allowing for better scalability. Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with "Pull Request" or "PR" for readability.
Add a section in CONTRIBUTING.org mandating a changelog entry in every PR. This is done to share the workload of maintainers and streamline the merge process as suggested by @kain88-de in syl20bnr#8686.
Based on discussion in syl20bnr#8686, introduces a structured PR review process that community members can follow. The PR process currently borne by collaborators is split into two phases, review and merge, that can be completed by different people thus allowing for better scalability. Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with "Pull Request" or "PR" for readability.
Add a section in CONTRIBUTING.org mandating a changelog entry in every PR. This is done to share the workload of maintainers and streamline the merge process as suggested by @kain88-de in syl20bnr#8686.
Based on discussion in syl20bnr#8686, introduces a structured PR review process that community members can follow. The PR process currently borne by collaborators is split into two phases, review and merge, that can be completed by different people thus allowing for better scalability. Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with "Pull Request" or "PR" for readability.
Add a section in CONTRIBUTING.org mandating a changelog entry in every PR. This is done to share the workload of maintainers and streamline the merge process as suggested by @kain88-de in syl20bnr#8686.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid! |
|
Sorry for raising a 3+-year-old issue but is this still worth keeping open? |
There's no need to be sorry, it's helpful to get suggestions about issues that can be closed. |
and others
Branching discussion off of #2784
@fiveNinePlusR, @rgrinberg and myself have been having a discussion on that thread and I believe we may need to re-evaluate how PRs are merged. We have PRs that have been sitting for years at this point!
At current there are 1,171 open issues (1,172 including this one) and 339 PRs. This is far too much for one person to do as a side project.
There should probably be some more maintainers added at some point. I am concerned about burnout, it's not a pleasant feeling.
The text was updated successfully, but these errors were encountered: