agreed.
there are several examples.
There seems to be some misunderstanding here.
- Literally anyone with a github account can propose anything to HA core.
- Anyone can review and comment on those PRs.
- Anyone can be a code owner of items in core, even if they neither work for nabucasa nor are a core dev.
- You don’t have to get a dev to volunteer for anything.
- If you submit a new integration it is expected that you will volunteer to be the code owner. And you absolutely don’t lose control over it; probably you will submit most of the PRs. But you do end up agreeing to the project’s quality controls (test coverage, code style, architectural rules), and PR review which means a delay.
The restriction is that only certain people can approve and merge PRs into HA core. I hope all would agree that this is necessary.
The reason the core devs aren’t trawling through HACS trying to find new integrations for Core is that we already have a huge workload of PRs where people are contributing exactly that. There are no objections to new integrations being added, as long as they conform to the project principles.
There seems to be a perception that HA Core is a closed group where only certain people’s input is welcome. This is really not the case. Anyone actively engaging and contributing is valued and your help is wanted.
Where I’ve seen people come unstuck is when they get frustrated with the pytest requirements, architectural rules, or simply lose patience responding to review feedback. Yes, you might have to wait.
And back to the point, if those are the reasons the integration you want is in HACS not core, then you need to be happy with those reasons.
Remember that when you submit a 1000line PR you are asking someone to spend 1-2hours reviewing your work. Be kind, and be ready for feedback that might be quite direct ‘engineering talk’.
Please also review >=2 PRs first before submitting yours (as the PR template asks). This helps with the backlog but also with you understanding the reviewer’s mindset.
Is there a PR or issue raised for the ASUSWRT problem? If it’s this big one, it’s probably the sheer size that’s the main cause of delay.
5 Likes
From what I have seen from looking at the github discussions on PRs to various integrations submitted by their own original code owners, the reviews from the core team were often almost absurd nitpicking about irrelevant issues, in the lines of ‘you didn’t do it the way I personally like, so redo everything the way I want you to do it’.
Voluntary code owners are not employees of Nabu Casa. Reading through some of the PR reviews, they looked like a senior manager would treat a junior developer (and not in a good way). As a custom component developer myself, I found this extremely offputting.
3 Likes
This is exactly why a really independent plug-in mechanism is needed. For a healthy ecosystem, diversity is key. Having a select group of reviewers doesn’t promote diverse opinions.
Luckily we have HACS, which is providing exactly that. Would be helpful though if:
- HACS had a rating/review system
- HACS (or a similar functionality) would be standard part of HA
- The terminology around plug-ins wouldn’t be as confusing as it is now.
There’s a coding standard that pylint, mypy, black, and other linters can’t catch. It’s not nitpicking, it’s keeping the code in-line with core. Developers who attempt to add to core should be prepared to back up their code with reasoning. I’ve had to do it multiple times because the reasoning behind the code flow isn’t apparent to the reviewer. The issue is that many people who attempt to contribute to core do not have the patience to go through the process and get testy with any review. I’ve been there myself, but you have to remain calm and state your case (This is where people fall off the wagon). If there’s a contested section of code, it will take a month or two to get through the process. Contributors should be prepared for that.
3 Likes
Eh. While I don’t have any explicit examples right off the bat (I don’t keep tabs on those I come across), I call it gatekeeping and BDFL syndrome. A lot of open source projects suffer from that to an extent, but HA is particularly bad.
And this is bad why exactly ? More often than not, the original code author knows a whole lot more about his code and the intricacies of what it does (details about interfacing protocols, hardware APIs, whatever his integration is about) and has a lot more expertise on this than the reviewer. It is neither necessary nor efficient for a reviewer to fully understand the code. That’s not his job. His job is to make sure general code guidelines are followed (automated tools mostly do this), that the code doesn’t do anything obviously nefarious and that the general API interfacing rules are followed. No need to go into things like ‘I don’t like how you named this variable, change it !’ (and yes, I’ve seen this).
Understandable. It’s their valuable free time they have to spend on this process. Maybe this shows that there’s something wrong with the review process. Again, I don’t really have an example right here, but I found some of the the reviewers comments to be downright condescending, while they glaringly lacked the expertise to even understand what the original code contributor tried to explain. You obviously can’t generalize on this, and often review processes go smooth, but it still happens a lot more often than other open source projects I have contributed to in the past (some of them a lot larger than Home Assistant).
Tldr: some of the reviewers should show a little more respect towards the contributors, that would go a long way to make the process smoother and motivate people more to contribute.
1 Like
I’m not sure about you, but every company I’ve worked at has had coding standards. And every company I’ve worked at had nitpick reviewers that lived the standard to the letter. I’d always get other people to review the code. Unfortunately, you can’t do that in HA, it’s first come first serve (reviewers).
On it’s not bad, I advocate for reviewing. It catches things.
Well, naming should be explicit. While I can’t speak for what you saw, variables like hia should explicitly be hey_im_alex or something along those lines. Python (unlike other languages) is meant to be read like a sentence and easy to understand without comments. Basically, comment only when you do odd things that can be read but don’t make sense. If you can’t do that with your variable names, then you’re going to get some pushback in that regard.
Text doesn’t convey connotation. Most devs are robots, and robots sound like assholes. Just my experience. But yes, there are some bad seeds that sound/are more condescending than others. I’m not sure how to get past that, so I just take it as a personality trait and work around it.
2 Likes
Yeah well, if someone started to nitpick about my variable naming, that would be the end of my core contribution right there, to be honest. Not sure if there’s a solution to this. the HA core dev team is far too deep into the current processes to change anything at this point. Anyway. Ironically I have a code review myself in 20 minutes. I do get paid for it though, and so is the guy who wrote the code I’m reviewing (and I usually try my best to not be an asshole 
). When you do this in your free time however, perspective changes a lot.
1 Like
Heh, I wish all my code reviewers were like that. My past job… I had one guy who’s hands down the worse co-worker I’ve ever had to deal with. Worse than anyone in open source. I’m glad he was on another team… I probably would have quit (sooner) if he was on mine.
1 Like
The developer of HACS has given permission to anybody to use his code for any purpose.
See here: integration/LICENSE at 0d123355f227caa9e4b445e4b480e26a3a8e18e8 · hacs/integration · GitHub
Sure, I guess.
But the HACS developer is a member of the core dev team and he said he doesn’t want to add it to the HA core. So there’s that…
Crazy idea… would it be possible to involve that HACS developer directly in this discussion? Somehow it feels very awkward to only hear others state what he thinks.
Good luck with that. He doesn’t like to talk about it. If you can imagine, you aren’t the first person with this idea. You aren’t the first person to put pressure on the topic either, which is largely why he doesn’t like to talk about it. 
It would be an uphill battle and I’m pretty sure he’d put you on ignore before you started to hit the incline.
1 Like
How unprofessional
1 Like
The same could be said about your persistence
5 Likes
If you don’t like the review process, you could improve the experience by contributing friendly reviews.
IMHO there is a greater need for reviews of the existing PRs, than more PRs.
As for wanting to make HACS feel more built in, please understand that this is likely received as “We find HA core’s quality, security, and review process too annoying, so please give us something that lowers the bar of entry, even lower than HACS. Then please manage all the complexities, potential fallout and financial brand risk this creates and also endorse it.”
The answer will likely be no.
Maybe there’s a future where custom integrations have an official endorsed QA controlled process that is somehow decoupled from core, but you aren’t going to get away from reviews even in that case.
Faster, kinder reviews is a positive step. So you (everyone) can make a difference by contributing exactly that.
I fully understand all arguments and those are valid arguments. I only perceive it as condescending that the devs don’t descend from their ivory tower to contribute to this discussion themselves. If I ever considered contributing to HA, this experience definitely convinced me not to.
1 Like
It’s not the “devs”. It’s a single developer who doesn’t want to talk about this topic. That’s what you aren’t understanding . He’s well aware of this thread, he’s also seen how pushy you’re being. I wouldn’t say a word either.
1 Like
Thanks for being such a wonderful moderator, @petro ! You really made me feel welcomed. I’m going to mute this topic now. I’ve made my point and learned a lot about the HA community.
1 Like
Thanks, I try. It’s been great talking with you. I’m glad you’ve put the effort into to understanding the situation.
3 Likes