plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
447 stars 607 forks source link

Create a pull request template aligned with Plone policies #6223

Open stevepiercy opened 1 month ago

stevepiercy commented 1 month ago

See example.

https://github.com/Homebrew/brew/blob/master/.github/PULL_REQUEST_TEMPLATE.md

We also need to deal with pull requests for a class assignment. I don't want to tell them absolutely no, because there is a remote chance that we can engage the instructor or program director to either become a part of the Plone organization, become a contributor themselves, use Plone for a project, or make a financial contribution to incentivize review of their students' pull request. They need to give us—either volunteers directly or the Plone Foundation—something in return for what we already give freely.

I'm also looking at ways to fund work on individual issues, as described in https://github.com/collective/icalendar/discussions/697.

Please offer your thoughts, @sneridagh, @ichim-david and any other @plone/volto-team members, who are interested in this topic. Also @polyester who may be able to help us navigate the internal processes with the Plone Steering Circle and Plone Foundation Board.

stevepiercy commented 1 month ago

Plone Contributor Agreement related issue

Back in the days before we used electronic signatures for the PCA where we required inked signatures on paper, people were committed enough to Plone to attend Plone Conf in person and follow that process. Now most people who sign the PCA are not committed to Plone, and it becomes a problem for everyone. I think that vetting of applicants should take place. We need to define the criteria for vetting, where in the PCA application process we should vet potential contributors, and who would do the actual vetting. We need to run it through the Steering Circle, and possibly the Board as it may require modifying the PCA and legal review.

ichim-david commented 1 month ago

@stevepiercy @plone/volto-team Personally I think it's a good idea even if we make our own bullet points with only a few rules such as asking if they have followed the guidelines which we link to.

Have you followed the guidelines in our Contributing?

Have you added an explanation of what your changes do and why you'd like us to include them?

Have you written new tests for your changes? [Here's an example]

Have you successfully run brew style with your changes locally?

"We also need to deal with pull requests for a class assignment. I don't want to tell them absolutely no, because there is a remote chance that we can engage the instructor or program director to either become a part of the Plone organization, become a contributor themselves, use Plone for a project, or make a financial contribution to incentivize review of their students' pull request. They need to give us—either volunteers directly or the Plone Foundation—something in return for what we already give freely."

Personally, I'm -100 on this idea. As someone who has limited time at my disposal, I realize that my time helping Volto is best spent giving back to the community that has a vested interest in seeing Volto improve and who is willing to contribute. This project is supposed to be used for production environments, plone the "enterprise cms" as was the tagline which now becomes a classroom assignment for getting some passing grade. We should be welcoming professional work and even for that work we don't have any guarantee that we have the time to review it. If we look at our pull requests tab we have many opened pull requests that could use our review or guidance to get to the finish line and they are added by members of our community who are professionals who work with Volto and not students who simply try to learn React and as such think they are ready to contribute to Open Source.

I believe in our goal to improve our documentation, in having better ways of presenting Volto and how to perform real-world tasks with it so that the users are productive with it and turn visitors into users of Volto but not necessarily contributors which in my opinion should be done after having a good grasp of the framework.

I would rather see one good contribution after 1 year of someone learning and using Volto which is actually needed by that user and others in the community than 30 meaningless contributions that detract our focus from the work needed by our community which would only increase the detraction by the next meaningless pull requests that would follow.

ichim-david commented 1 month ago

I'm also looking at ways to fund work on individual issues, as described in collective/icalendar#697.

This I think is a good idea and we should make it easier to create projects for funding open-source work. We should be able to create through github sponsorship projects with funding goals for hard problems that need funding and let users dictate with their wallets on what is important for them by their contributions to whatever project they choose.

It's a fair system where you have contributors that do open source work usually funded by company needs or by pro bono work as paying back the community and then you have extra needs of other users or companies that can chip in and help a feature request get quicker done.

Unless we get over the tabu of open source and funding that is needed for it we won't get as far as we could if we would simply create a system of donations for specific needs.

ichim-david commented 1 month ago

Plone Contributor Agreement related issue

Back in the days before we used electronic signatures for the PCA where we required inked signatures on paper, people were committed enough to Plone to attend Plone Conf in person and follow that process. Now most people who sign the PCA are not committed to Plone, and it becomes a problem for everyone. I think that vetting of applicants should take place. We need to define the criteria for vetting, where in the PCA application process we should vet potential contributors, and who would do the actual vetting. We need to run it through the Steering Circle, and possibly the Board as it may require modifying the PCA and legal review.

I 100% agree on this issue, I am sorry to Steve for venting on this issue too long to him instead of seeing if something can be done. I think I had my CLA signed when I went to the first PloneConf at Arnheim and it was a great moment for me to get on the sprinting after I have been working for quite a few years already with Plone.

Making something harder to get gives it more value, if everyone can simply join the plone github organisation by sending an email it makes this membership seem less and less meaningful. We used to be only allowed to the community organisation and encouraged to work on add-ons, practice get experience and then see on how we can contribute to core if the ideas are solid.

Now we simply say that anyone is a contributor to open source and I think this is wrong as it won't attract meaningful contributions that are long lasting beyond a pull request or two.

When we go back to the idea that Plone and Volto are serious open-source products that are used by it's users to make money and that it matters that the code remains as high quality and bug free as possible then we will greatly vet what kind of contributions we are willing to accept and even bother to check. By definition outside of a GSOC project where there are maybe hundreds of applicants vetted before 3 or 4 are selected to be part of GSOC and in the end, something comes out of it, in the same way, we should vet all of these contributor agreement signing and choose only 3-4 out of the whatever number asks to sign it after we see that they follow our community guidelines, in particular, these points:

If we make a list of the last 50 students accepted we can look at their github profile and clearly see that they have failed to not do the things that we asked them not todo or haven't done what we asked them to do. The same will happen for the next student that wants to sign the CLA which by definition will fail to meet the criterias that we want from a contributor so we should stop giving attention to these pull requests and not accept the signing of the CLA.

Lets try to avoid giving burnout to core developers by increasing review demand for low quality contributions when we could instead focus our attention on the work we have already waiting to be reviewed from the members of our actual community that come again and again and try to contribute quality pull requests.

stevepiercy commented 1 month ago

For the PR template, I would like to add:

I want to be careful that we do not deny all contributions just because it is a class assignment. Newcomers have the best perspective to see that our installation, contribution, and developing procedures are incorrect. Without these first-timers, we would still have incorrect and inconsistent documentation. The existing contributors "just know stuff" and rarely refer to documentation to do work, and less so often to check for its accuracy.

I also want to be careful of any barrier that we impose which would block valuable contributions. Because the Pylons Project and Pyramid folks were so welcoming to me, I started my journey to learn Python, Pyramid, and all of its related technologies by trying to install it. I submitted my first GitHub pull request to improve Pyramid's installation documentation. My motivation was for career development, as I could see that the proprietary commercial closed source language with which I began my career was dying. I was also motivated by the welcoming open source spirit prevalent across Python, the Pylons Project, and Plone.

There are also some projects that I use very rarely, but when I notice something incorrect, I will submit a pull request or an issue to correct it. I then move on, and often never go back to that project. I would not want to block these kinds of contributors.

ichim-david commented 1 month ago

For the PR template, I would like to add:

* [ ]  This pull request _is not_ for a class assignment. If this pull request _is_ for a class assignment, then your instructor must either (1) fork this repository and review your pull request against their fork, or (2) contact [insert appropriate Plone entity here] to arrange for review by contributors to this project. Pull requests that improve documentation only and do not include code are excluded from this requirement.

I'm ok with this message, a bit doubtful about the contact to arrange for review but otherwise it's fine.

I want to be careful that we do not deny all contributions just because it is a class assignment. Newcomers have the best perspective to see that our installation, contribution, and developing procedures are incorrect.

How would a newcomer have a better perspective than we have on our developing procedures? Yes, a newcomer could check the docs and add an issue if they don't understand something and we could check that issue and assess if we need to improve the docs or if it was simply human error. In anything having to-do with installation, contributing, or developing guides it's on us to improve the docs after we get issues and not by a newbie in my opinion.

I also want to be careful of any barrier that we impose which would block valuable contributions. Because the Pylons Project and Pyramid folks were so welcoming to me, I started my journey to learn Python, Pyramid, and all of its related technologies by trying to install it. I submitted my first GitHub pull request to improve Pyramid's installation documentation. My motivation was for career development, as I could see that the proprietary commercial closed source language with which I began my career was dying. I was also motivated by the welcoming open source spirit prevalent across Python, the Pylons Project, and Plone.

The docs are another matter altogether, for which I agree that we could always use some improvements. We should encourage contributions to docs for which there is no need for the CLA signed. I suspect it won't be an easy task though as you need to know the framework first before knowing what to write about it in the documentation so this kind of excludes these newbies until they go through maybe the Plone and Volto training docs. I wish more would test those and add issues or feedback and pull requests if something is old already and needs updating.

There are also some projects that I use very rarely, but when I notice something incorrect, I will submit a pull request or an issue to correct it. I then move on, and often never go back to that project. I would not want to block these kinds of contributors.

Fair point and indeed we have many issues that could be worked on if an individual has the knowledge todo so. Depending on the usefulness of that pull request someone from the core volto team or from the volto community could choose to review it. This is why I want to propose that we use the "32 needs: review" label for these pull requests but not to tag the plone/volto-team asking for a review.

If someone from the team is willing to review something they can see what pull requests are opened and have this label and pick the pull request that makes the most sense for them to review. After all "Open Source maintainers don't awe anyone anything just because an issue was raised or a pull request was added", see this post that the Homebrew project leader wrote https://mikemcquaid.com/open-source-maintainers-owe-you-nothing/ If we always flag the plone/volto-team for everything it will end up feeling just like spam with countless notifications where someone asks more out of us. This makes it hard for us the core contributors to get actual feedback from the group when we do use this tag because no one bothers to read the emails and reply when we ask something.

Since we are all volunteers in our giving to the Plone open-source community we also need to clearly state this to these newcomers, that we have limited time and availability to work on whatever pull request we deem important. This way there is no false expectation from any side. If someone really has the desire to contribute to Volto and they do so and someone steps up to review then there is a good chance it will end up merged, otherwise, it won't and if you think that you can pad your stats by contributing to open-source then I don't think that Volto is or should be the way to get there.

I did my fair share of reviewing and helping these newcomers with the idea of trying to free up the time of the other members of the Volto core team. Seeing how little time we have available and willing to spend especially now during the summer holiday season made me realize that I am doing the Volto community a disservice by not focusing on the pull requests already added by our constant contributors which are part of the community and have their pull requests go to the back of the line possibly having code rot due to other merges simply because I've focused on merging the one pull request and gone type of individuals. That is not fair to the time I spend and to the community that has pull requests still waiting to be merged. As such my personal decision is to focus on what I can reviewing community pull requests leaving the newcomer work to whoever is willing to review it and not asking anyone to-do like me or to review that work because again this is all unpaid work done on a volunteer basis.

The best plan would be to work on pull requests that are most useful to the entire community but this also depends on what that means due to the potential of many interest groups when having such a big framework like Volto (ex: performance, a11y, i18n, extensibility, etc). If more people get involved in the reviewing process there is a greater chance something gets merged, if not, fewer things will get merged, and pressuring people to review some work will not work, the reviewing needs to come naturally out of a desire to help more improve Volto for all of us that use it.

stevepiercy commented 1 month ago

In anything having to-do with installation, contributing, or developing guides it's on us to improve the docs after we get issues and not by a newbie in my opinion.

I vehemently disagree with this statement. It is not true for me, as I previously explained that my first contribution as a newcomer to both Python and Pyramid was for installation docs on Mac OS X (now macOS X). Imagine how slamming the door on me back then because I was a "newbie" would have impacted my contributions to Plone today.

I also have reviewed several pull requests to these processes by newcomers, some of them by drive-by one-timers. In my experience, newcomers are more motivated to improve these guides than the experienced developers by submitting issues and pull requests, and asking questions in Discord and the Community Forum. Experienced developers lack the fresh eyes and don't see problems with the documented processes, or they "just know stuff" and don't refer to the documentation. I personally don't care who gives a contribution as long as it fixes or improves that which could be better.

Changes to code are more challenging to qualify. We currently have one PLIP that was a GSoC project from 2023, has negligible value to Volto, and we are currently getting flooded with pull requests that we do not have the time to review. No other issue is generating low quality pull requests. This is bad communication. We can solve this problem by either closing that PLIP, assigning it to an individual, providing clear expectations to those contributors that their pull requests may never be reviewed, or a combination of the foregoing. We still are not clear about who may work on that issue. At least @sneridagh replaced the misguided statement of "Anyone who would like to help will be appreciated.", but we still need to be absolutely clear of who should not participate based on their expectations.