More maintainers and reviewers #1730
Replies: 10 comments 72 replies
-
|
In general, every contributor is allowed to provide Reviews to Pull Requests. It may not provide the "green checkmark", but the maintainers still take them into evaluation for merging. Besides that, we try to not downright reject changes, but try to find ways on how the desired goal can be reached. Except its a major Security or BC Break. |
Beta Was this translation helpful? Give feedback.
-
|
@fballiano Alright will check it out in a few hours. Thanks. |
Beta Was this translation helpful? Give feedback.
-
|
Reading the current RFC for accepting PR's I can say it is not wrong either, but too many PR's have remained open for a long time. Here is the big question, when a PRs is good, complete or unfinished? I do not want any haste in their approval because at least in the last half year I woke up that either a file was accidentally deleted (color picker), the installer did not work and many other issues that had the approval of some people with the experience. Those who approve must check that PR not just click on a link because they think it was okay to read the code. Those of us who have time should take all 180 PR's in hand and express our opinion if they remain open, closed, or being integrated into OM. Here is the big issue the reaction time. And some PR's are even complex and require attention, but what happens to them must be established once and for all in the next months. I also look at the activity of the authors on GitHub when I try to contact them in their PR's. In other words, good PR's can be taken over by other authors and carried on. |
Beta Was this translation helpful? Give feedback.
-
|
@addison74 that is why I'm trying to define a RFC to change the rule. bugs will happen, OM will never have the power of a corporation, that doesn't mean we've never change anything. the v19 is the stable one, v20 if you want improvements. something will break, everybody is careful, everybody here have multiple OM projects in production and nobody wants to see them broken. Anyway I think a PR is complete when is approved. |
Beta Was this translation helpful? Give feedback.
-
|
following the RFC rules I had to change the proposal address to https://github.com/fballiano/rfcs/blob/main/draft/0000-change-pr-merging-rules.md I've also drastically updated it (introducing a minimum number of merged PR to be part of the approving users group), any contribution is welcome to define something that works for everybody and solved the problem. |
Beta Was this translation helpful? Give feedback.
-
|
A level of priority of applying a PR should also be considered. Some need to be applied urgently and others may wait. There were situations this year when new versions were launched and issues were reported in the same day but they were not solved immediately, some being fixed even after a week or more. I am talking about issues that affect the current functioning of OM and that cannot be postponed. |
Beta Was this translation helpful? Give feedback.
-
|
@fballiano Thank you so much for your RFC. Currently, OM has only 1 merge criteria: 2 green checks. The grey checks doesn't count, even though @Flyingmana says the greys add weigh to the PRs for it to get merged. Your RFC proposes to add an additional merge criteria, 1 green + 1 grey checks. I support this. However, I would prefer to have 1 green + 2 greys. This would delay the merge a little bit but grey reviewers are more active anyway in OM, so it shouldn't cause too long a delay. So, if a PR has 1 green + 2 greys, then it meets the merge criteria and should be merged. Does that make sense? Further, I would propose that a PR with 3 greys without anyone objecting should be considered as a third criteria. |
Beta Was this translation helpful? Give feedback.
-
|
@fballiano Awesome! I support your RFC. Thank you. |
Beta Was this translation helpful? Give feedback.
-
|
I don't think openmage need to change the merge rules. I think more users should be privileged as maintainers. I belive there is many companies running openmage. One or two people from that companies should attend as maintainer in openmage and push forward their priorities. Otherwise they must be waiting to other maintainers that are doing their job for free (see my post about sponsorship below), which take a lot of time, because people tend to do first the things that thay are paid for, later the free ones :) |
Beta Was this translation helpful? Give feedback.
-
|
I think that there should be a "rule" on how to be(come) a maintainer, is there now? |
Beta Was this translation helpful? Give feedback.
-
|
I think there isn't. I became maintainer just after some discussion with flyingmana. |
Beta Was this translation helpful? Give feedback.
-
|
Maybe there should be a submission form to apply to become a maintainer and X number of current maintainers need to approve it? |
Beta Was this translation helpful? Give feedback.
-
|
For that reason, I think openmage should start some sponsorship program. Companies that love this project would be paying for it. Even our company would be paying for it. Then the maintainers would be motivated much more to push things forward :) |
Beta Was this translation helpful? Give feedback.
-
I am one of those merchants. I am happy to contribute to this project, but also I would prefer to do it anonymously if possible. I am happy to be on the private record but would prefer my name/business redacted from public viewing. I think most merchants (not all though) would want this. But this seems to be 2 different ideas here:
|
Beta Was this translation helpful? Give feedback.
-
|
@Dirtyhair1 for many of us, there would be taxes involved and possibly additional fees by taking the money into an account which is assigned to us personally. PayPal would be a terrible choice for this, as they like to close these kind of accounts, or block the money flow for months. |
Beta Was this translation helpful? Give feedback.
-
|
@Flyingmana yes I understand what you mean, I just mentioned Paypal but now understand it's not a great idea. So what are you thinking here? I also maintain my comment above: "But this seems to be 2 different ideas here: Which are you thinking about/suggesting? Or both? |
Beta Was this translation helpful? Give feedback.
-
|
@Flyingmana ok I have been thinking about this a bit more and now I understand what you mean about taxes etc. I agree an entity would be best, also better for the long term. There could be 2 separate accounts. Those who want to sponsor for bug bounty could donate into one account, and those who want to sponsor for other purposes could donate into another account, that’s my first thought at least. |
Beta Was this translation helpful? Give feedback.
-
|
I wonder if something like the route Home Assistant went could work for OpenMage? Basically they started a for-profit that extends the project (Nabu Casa). With the funds raised from that feature, they have brought on developers full time to enhance the open source project and allow them to concentrate on it full time. |
Beta Was this translation helpful? Give feedback.
-
|
It is to me also not entirely clear what is being allowed into the master. I think a decision should be made whether or not magento-lts is moving forward or just receives security updates. Eg. the default template requires an update Eg. apcu is this now added? Eg. allowing different email templates to be stored within theme location. Eg. allow recurring payments for bank transfers |
Beta Was this translation helpful? Give feedback.
-
|
I alreay knew that recurring payments were always in beta stage and never fully completed. |
Beta Was this translation helpful? Give feedback.
-
|
Although I am not really familiar with the magento structure, I have 'I think' something working with recurring payments for the bank transfers. Just some proof of concept without any exception handling etc. |
Beta Was this translation helpful? Give feedback.
-
|
I see the magento-lts as the entry level stores for people that do not want(need) to install magento 2 + elastic search + ... . any other crap (yet). It is good for small solutions, and does not require a large footprint. In that line of reasoning, one should offer the apcu it is just a quick win, without having to need to install some 3rd party solution. The same is for this issue with these email templates being grabbed from wrong location. The whole idea of a smooth and fast installation is to unpack the magento-lts source, dump your theme and other modules in it, and preferably run a group of n98-magerun.phar commands and you are up and running. Maybe it is good to send some questionnaire to see what the magento-lts audience is like. |
Beta Was this translation helpful? Give feedback.
-
|
I have been with OpenMage for almost 2 years and I can say that this project is based on the contribution of those who still use the Magento 1 platform. It is a voluntary, unpaid contribution. At this time 71 people are receiving notifications about changes made to OpenMage. There is a list of issues, one for PR's, and another one for discussions. Although there are many stores that still use M1, those who manage them do not participate here. Otherwise we would see an increased interest. APCu is the successor to APC, in the past I used it when PHP was at version 5, but starting with version 7 the Zend OpCache extension is the one I use. The same thing happened using Redis, all the sessions are stores in the system file. In addition, I have a dedicated server like I didn't have in the past with NVMe, RAM, a lot of processor cores. I could use Varnish cache but for the moment all stores I managed are loading the pages fast. I can deal easy with 200 simultaneous connections and for DDoS I am using a hardware solution for which I pay monthly subscription. I doubt you'll find someone to bring APCu extension up to date. With a few exceptions, most developers have taken their extensions out of sale and services for M1. I don't know how long OpenMage will last, but it is the result of the contribution of each of us, and I'm glad there are still people interested. I evaluated a lot of shopping carts like Woo, Shopware, OpenCart, Prestashop. I'm ready to migrate to Magento 2 when I'm forced to. We will see what issues the new PHP versions will involve, here is the key to the survival of M1-based stores. |
Beta Was this translation helpful? Give feedback.
-
I already showed some example of how to update to apcu (it is sort of working in my environment), I am not eager to submit pull requests because of my inexperience with magento. I think some experienced developer would generate a more efficient result. |
Beta Was this translation helpful? Give feedback.
-
|
The advantage of having more maintainers is that a PR can be edited/modified by them once issues are reported. If we look at the number of PR's there are so many in which some of us requested revision or update but we did not receive any response from the authors. If there are more maintainers then you can increase the number of approvals by a PR needs to be merged. We have to do something quick. We have been talking about PR's for almost 2 years and things are not moving at all. We should decide what to do with each PR periodically. I think that if we make a plan we can solve the vast majority in a maximum of 3 months. |
Beta Was this translation helpful? Give feedback.
-
|
I lost the link of @fballiano that list PR, but there are some PR with some "gray approve". |
Beta Was this translation helpful? Give feedback.
-
|
@luigifab - check this out https://github.com/fballiano/rfcs/blob/main/draft/0000-change-pr-merging-rules.md. At the bottom of the page there is a link. |
Beta Was this translation helpful? Give feedback.
-
|
Another advantage would be that keeping active maintainers can debate closing a resolved reported issue or without feedback. In the Issues section today I saw a few posts that have solutions, the authors are asked to provide feedback or close them, but no reaction. We need to do a little cleaning after a while. |
Beta Was this translation helpful? Give feedback.
-
|
Hi everybody, I've just posted the PR for the RFC here: I don't know how many maintainers or the RFCs repo are reading this but for it to be merged there's the need for the approval of 2 maintainers. 🤞🤞🤞 |
Beta Was this translation helpful? Give feedback.
-
|
Is something now changing here for the better or not? I do not even get why this apcu is still not added, why is nothing being done here? If it is not possible to get 2 maintainers to approve something, then switch to 1. |
Beta Was this translation helpful? Give feedback.
-
|
I think we are not obligated to do something with them (like I not obligated to answer issue/pr for my open source repositories). |
Beta Was this translation helpful? Give feedback.
-
|
exactly @luigifab, I've been try to say the same thing for months and months, with the current pace the project will die. let's face it (as we all are M1 developers and we are here cause we love it) nowadays M1/OM it's only used for old projects that didn't want to migrate to anything else, but for new projects everybody goes to shopware or shopify or something else... |
Beta Was this translation helpful? Give feedback.
-
|
I don't think it makes sense to justify our actions. I personally appreciate the fact that there are still a handful of people who contribute regularly and that's because they maintain M1-based stores. To be well understood, M1 is still a very high-performance, mature platform, I have come across custom implementations made by absolutely fabulous developer teams. As long as this project continues it is all that matters. I'm sorry I haven't seen posts from important people in a while. |
Beta Was this translation helpful? Give feedback.
-
|
I contacted flyingmana with list of potential maintainers. He told me that he know the names and will hopedully make some of them to be new maintainers in week or so. |
Beta Was this translation helpful? Give feedback.
-
If some copanies wants to merge PR quicklier, they can send money to the maintainers. For example @Flyingmana have sponzorship program and If you want you can send him money and he will spent some time with your pull requests. I think, that all maintainers here are great experienced M1 developers, but we do not have a time to spend with OM 2 hours a day. Me and @midlan thinks, that we should invite more people to maintainers team. |
Beta Was this translation helpful? Give feedback.
-
|
:) :) I am glad to see this discussion, you can almost call it progress!!!! :) :) Is there not someone that can add an extra shop where magento maintainers/developers can create products (from PR's, feature requests or whatever) that a sponsor then can buy. But keep in mind such intermediate that facilitates the shop, does not get caught in some dispute when a feature is not implemented as the buyer expected. |
Beta Was this translation helpful? Give feedback.
-
|
Hi everybody, https://github.com/OpenMage/rfcs/blob/main/proposal/0001-change-pr-merging-rules.md was merged in the RFCs repository, but it needs votes to be "accepted". I've opened an issue OpenMage/rfcs#5 to allow you to vote for the approval of this long awaited change! Thanks everybody! |
Beta Was this translation helpful? Give feedback.
-
Hello everyone,
In my company, we are currently in the process of migrating to OM with PHP 7.4 from Magento CE with PHP 5.6. Because of how much time and pain this project saves us, I lately decided to start contributing to this project and giving back to the community. I'm very happy this exists.
The problem that I'm noticing is, there is a lot of pending Issues and Pull Requests, some with no reviews, some with partial reviews... etc.
I understand that the maintainers have other things in life, but my suggestion is to get some other trusted people that can review and approve/reject changes.
✌️
Beta Was this translation helpful? Give feedback.
All reactions