I wish I could spend time helping in some meaningful way, just not sure what skill level is required
Having reviews from anybody interested would help, cause time is rather limited and then its really hard to review stuff that I am not familiar with.
So, please anybody that has an understanding of what its supposed to do please review PR-s that are interesting for you, even testing them and posting you Tested-by really helps.
5 Likes
As a person that hardly knows what he is doing or how stuff works around here I have managed to create a pr and pull request for a new led driver, but I don't specifically know whose area of expertise it is to be able to ask anybody to take a look at my shitty coding attempts.
I have no idea which devs are willing, wanting or capable of commenting or reviewing the area under the umbrella my code slots into.
I pm'd a few people on the forum and most said in short terms I don't know much about the led stuff sorry I can't help, or you get no reply, and I can see most devs want to be doing stuff that interests them so it would be nice to somehow assign the correct person or persons to the bat that would be enthusiastic to work on it.
Many of the devs are not on the forums or their primary method is email, mailing lists or irc that many people in today's gen don't even know they exist.
But you can imagine from my perspective I really don't want to ping people on the forum and as such I feel like I'm hassling them.
I left my pull request in draft for so long during testing that now it is so far down the list that that it is probably overlooked as being old hat.
So I'm stuck in a situation where I don't even know who to contact to progress further, my linked pr in reality does not affect anybody as it is not a core package, and it passes the build process it in reality could be merged without six separate reviews.
1 Like
Isn't that the second version of the PR/driver?
In the first one I remember requesting that it be upstreamed first, we have too much code that is just left in OpenWrt and never upstream (Hence why kernel version changes take so long)
2 Likes
Yes it is the same PR/driver I want to upstream it but in reality, I don't really know what I'm doing I still trying to figure things out as you are aware I'm no dev just someone with a lot of free time to mess around with what I think is interesting.
I just want someone who would be able to review it that would be able to comment that I would not be wasting my time trying to upstream it and get shot down in flames.
I guess I just need to try create an upstream patch I tried to join some of the linux led mailing lists but I could not figure out how to get it to work, I guess I have been reading to many rejection posts on the net with bad submissions.
I mean, what is the worst that can happen upstream?
You will get no reply or somebody who actually knows a thing or two about the LED subsystem will review it.
4 Likes
@systemcrash and @robimarko Don't you guys think that a more systemic approach to solving this is warranted at this point? Such as: appointing someone to actively engage with the community in order to get code reviews where they are needed, weed out abandoned PR's etc etc. IMHO this is important enough to even hire someone, but if there are no funds then at least try to find a volunteer? I mean no disrespect (in fact, I very much appreciate your work) but it seems that the current approach is not good enough, and doing more of the same is not going to change that.
2 Likes
While I get the idea behind your suggestion, I dont see how that would help get reviews by itself as PRs are public, so unless that person would start tagging potentially interested people, I dont see how it would improve the situation.
That being said, we for sure dont have the funds to hire somebody.
We sadly just lack the manpower to respond in a timely manner, especially for more obscure parts
1 Like
Upstream first. This is a good example of how it's done. Send a patch-set and await feedback.
I'm not sure how much more active we should be than we already are. Most everything is public. I think @robimarko has a full plate - getting pinged every 17 seconds in about 900 repos 
. About the only thing I might change is give some more contributors commit permissions. There are few generalists who know everything, but there are lots of individuals happy to focus on their corner of some part of this project. See also:
Just gotta ping users for reviews - even teaching them how to do it.
Perhaps, there could be a help needed tag for PRs that need assistance with reviewing, testing. Right now I review new PRs in packages from new contributors to make sure they look merge-able, i.e. follow contributing guidelines, don't fail basic shellcheck (by eyeballing for the most common issues), don't use packaging hooks for random things, etc. But most often original contributors disappear, so there might need to be a better process to handle stale PRs, e.g. a bot pinging contributors, and closing PRs when not response has been received withing a certain time frame.
1 Like
Can a bug bounty system be setup where people can incentivize users to work on stuff that is important to them.
There is plenty of current platforms that do it now, I have paid for patches for retroarch a few years back.
3 Likes
How about this:
- make a forum thread titled "Let's get these PR's resolved - help wanted!"
- List what needs to be done and what skillset is required
- Appoint someone trusted to monitor the thread and organize the work of people who volunteer
(Optional: establish some mutual reward system where people review a PR and in turn, get their PR reviewed)
IMO a lot can be done with help of the community, someone just needs to actually engage with it and organize the work.
2 Likes
I suppose nothing really stops us from doing this already. This can be split by repo, as in:
- Reviews wanted, base
- Reviews wanted, luci
- Reviews wanted, packages
- etc.
This could be a template.
1 Like
This could be a grassroots thing but I think that this will be much more effective (in terms of people responding) as an initiative of the core team. Let's see if they pick up the gauntlet.
Not sure having a quid-pro-quo system is a good initiative - nothing stops people pinging here to ask for a review, just as a few have done in this thread. Generally pinging or '@'ing someone who you think has the skills to do a review is already SOP. But making that obvious by adding it as a line in the github PR template file could be a good addition.
I'm not sure either, that's why I marked that as optional. What about the other (main) idea which I think you didn't address?
1 Like
in currently going through the pr process myself, I think what may be most beneficial to the review team and the developer alike, would be to have a pre-pr process. Use the forums to present the pr before starting the process.
I my case anyway, a lot of things should have been done before even starting the process. Much of which would've likely been caught, for lack of better words, by the regular members or members that have been through the review process.
This would also provide the core team with a look at what is coming down the pipe and afford them the chance to point out obvious corrections/additions that should be made before the review. And beyond that, provides a glimpse of the intentions of the pr, as to what it does, and how well its been tested .. received.
This could also serve to spotlight those that are most eligible to join the review team .. judging by the most active and engaged pre reviewers and there perceived skill level
3 Likes
Thanks for the input. Can you go into the specifics of what you think could've been done before the PR?
1 Like
my case may be a little unique ... much to the displeasure of the reviewers, my pr turned into more of a development discussion then the normal pr.
to elaborate a little, I added the uboot support for my device after the pr was started, after some good points were made by a reviewer. This had kind of a domino effect as more and more things were needed to add the uboot support and maintain normal upgrade functionality. As you can imagine, this ongoing development made it next to impossible to review, things were constantly changing drastically from review to review (sorry @testuser7).
Looking back on it, I probably should've closed the pr and went back to the forums to finish development. But by this time, all the pertinent information was tied up in the pr ... +400 comments and all the testing data and logs.
Now I sure hope this isn't the norm, but I think to some degree it is, as in pr being started with obvious errors and or key pieces missing ... incomplete if you will.
Another key thing i learned was, even though the original pr worked and I was more than happy with it at the time, it is just as important to take into consideration the needs many compared to your own from the start.
To put it simple, I think it comes down to the more polished the pr .. the quicker the review and the happier the reviewer/s.
3 Likes
My two cents regarding PRs... I think there is a big lack of documentation, or documentation is outdated/incomplete, and OpenWrt could use more standardization.
For example, adding new devices. The mantra is 'just look at some previous examples, and figure it out from there'. The problem with 'look at some examples' is that there is not enough uniformity across (mainly) DTS-files. I see this lead to low(er)-quality PRs from less-experienced community members. Such PRs require more time and effort from reviewers, multiple iterations, and that can lead to committers getting demotivated and PRs turning stale.
If there would be better documentation on how to write a DTS, and explain why some code is the way it is, this would imo lead to more engagement and better commits.
I understand that documentation is the least sexy part of coding, but it's also the lowest-effort work for the community to contribute on.
2 Likes