WTH, why does it take more than half a year to get your PR reviewed by a core developer?

I hope it helps.

I’m not sure it will, it looks like you’ve updated that to a beta version of the library that the integration uses? Typically that’s not allowed if I recall correctly.

You might want to inquire over in the developer channel on discord.

I am an integration code owner, and it has been really frustrating to wait multiple months for a review. It adds extra work/time because I end up going in multiple times to re-base code.

Does it make sense for integrations to live both in Core and in HACS ? So HACS would have the latest and greatest, then a few times a year, that gets moved over into Core? This would cut down on the number of core PRs, and allow for faster updates for integration updates.

I’m not sure if relying on hacs would work. The issue with that is that if anything goes wrong, the developers can just say “well you’re using a custom integration that’s not supported”.

I think a better idea which has been suggested elsewhere is to offer an “LTS” version of Hone Assistant that will be guaranteed to work for some set amount of time so users have an option to not have to contend with breaking changes every month. Then they could contine offering non-lts releases so users can choose to get more frequent updates, versus long term stability with less frequent updates in sn LTS version.

3 Likes

While I fully agree that an LTS release is absolutely needed for HA, and would solve so many issues, I’m not sure it would help with the issue @austinmroczek mentioned. The problem here is that once your code is in Core you can’t update it anymore without approval from Core devs, even if it’s your own code. And since the review process can take so long, you end up being unable to update your integrations for months.

To quote the HACS guidelines, ‘Custom integrations that override core integrations will not be accepted’. So you could not publish it as a default HACS repo. It might actually be better to remove your integration from Core altogether and publish over HACS only.

2 Likes

I see what you mean, LTS might not fix this specificly. Sometimes an integration could need an update right away, like if it relies on an API the vendor changes (like how myq changes things constantly). Like a lot of things, there’s advantages and disadvantages either way of having an integration in core vs hacs. If something is in core, the beuracractic process for changes should make sure the integration fully works with the core program, but it definitely does slow down the process for updates.

Versus hacs though, where core developers have little to no regard to whether the hacs integration works or not when a change is made. When a core change is made, it’s totally up to the developer of the hacs integration to adapt to whatever change was made to keep it working. When a change is made that impacts a hacs integration, the core developers can just say they’re not responsible for custom integrations. This happened recently to the custom nws integration. Core developers made a change and the custom developers had one release to fix it before their integration stopped working.

So ultimately, I’m not sure what the best way around the slow update process is, but there seems to be downsides to having a custom integration vs an “official” core one.

Hotfixes are one thing, but this goes even even further. @austinmroczek mentioned that his core integration commits waited for review for multiple months. Imagine having added lots of nice features to your integration many users are waiting for, ready to be deployed. But you can’t, because noone approves the changes to your own code, and you sit there watching as multiple HA releases go by. That can very quickly kill motivation.

Yeah. Experienced that myself. The breaking change that broke my code was mentioned in some devblog, which I didn’t read. It’s annoying. I don’t really know what the perfect solution would be, if there even is one. Maybe a well maintained plugin API, tied in with an LTS release would work (that’s how most commercial applications do it). But as far as I understood from other discussions about this topic, that’s not something the HA core devs are considering. Personally I am going to keep my components custom.

I know the core folks are trying hard to get everything done, but the great success of HA has made it hard to keep up.

One option might be to only keep the most used integrations in core, and push most others out to HACS. Set some threshold in order to get “promoted” into core. Maybe # of users or % of total installations.

That would reduce the number of PRs for core team to deal with, and would let smaller (by number of user) integrations continue to evolve quickly until they are widely adopted. It would also ensure the most used integrations are still tightly integrated with core and well tested so we don’t scare away the “average” users with lots of errors.

To go with this, we would need to minimize changes that would break stuff in HACS, as was discussed above.

1 Like

Yeah, I’m still waiting on a core developer to take another look a the PR.

There are two community testers that have run the integration as a custom integration. They both have approved the PR (which is small). This is very frustrating to say the least.

Too busy developing the year of the voice with stuff almost nobody is waiting for :neutral_face:

7 Likes

It might not be something you want personally, but I don’t think think it’s a case of “Nobody wants, nobody needs”.

1 Like

I got a “stale PR” message on the documentation PR. Stale because I can’t get a code maintainer to look at the parent PR. :disappointed:

This PR process has really soured me on contributing to the project.

I was originally excited to give back and help solve a problem that I and others in the community were facing. And I was really excited when I figured out the solution and was able to jump through all the hoops for submitting the pull request.

Now I am frustrated that the PR just sits there … waiting … for someone with the ability to merge to look at it. In total, it has been about 5 months since I first submitted it, and about a month since I updated it based on feedback. I just want it merged so that I can stop thinking about it.

It will probably be a long time before I will try to get something into HA. I wonder if HACS is more responsive. :thinking:

5 Likes

Same thing on my side, for the past months I have been moving over to HA from Hubitat (HE) and the PR review / reactivity has not been the best experience.
I could even get things done faster and implemented in a closed source solution like HE, where the team was way more reactive and responsive to features and enhancements from the community.

Let’s take as an example this PR (95053), it’s been more then 4 months and I am just counting my PR not the original one just to add support to a new device in an already existing integration.

I really thing the community could all benefit from having more then a single approver (bottleneck), it would be great to have a minimal of two or three approves per integration or component.

1 Like

I hear the sentiment of what everyone is saying here. I have a foot in both camps by reviewing a lot of these PRs and also waiting sometimes >1yr for my own PRs to be reviewed.

My best advice is to keep your PRs short. A short, simple PR will get reviewed more quickly. It raises fewer questions, has lower risk, and is much less likely to become complicated or conflict with anything else. Less than 100 lines automatically gets the ‘short PR’ tag. If you can break a 600 line PR into 10x 60line PRs, it will be way quicker, as you are not asking for such a bulk time commitment from the reviewer.

Second piece of advice - put yourself in the mindset of the reviewer. They are just another human. Make the description really clear, explain what you are doing and why. Don’t give them lots to figure out. If you are doing something that might break things, describe how you’ve mitigated the risks.

Lastly - be the change you want to see. While you are waiting, review some other people’s PRs. They will value it and often return the favor.

Pinging your PR or tagging people is not a good idea unless there is a specific reason for urgency such as something else being broken. Many reviewers will de-prioritise overly pinged PRs to avoid encouraging this behaviour.

In case anyone is not aware, this is the list of open PRs for HA core (currently over 500), the default sort is newest first. I recommend looking at where your PR is on this list before insisting that you’ve waited an unreasonably long time.

Thank you for your contributions (and your patience!).

2 Likes

Respectfully, your post comes off a bit “privileged”. Have you ever owned/maintained a project as large as home assistant with this breadth of integrations without expecting compensation? Some things I’d counsel you on personally:

  1. This project has a high amount of PR contributions.
  2. Your personal PR isn’t special just because you wrote it.
  3. Your personal PR isn’t “free code”. It will have to be modified to be better quality, potentially change design, etc. If you’re a developer you should know PR’s aren’t the finish line. They’re the half way point. You have a proposal that takes work to be production ready.
  4. How small are your PR’s?
  5. How good is your code?
  6. Have you looked into what you could do to start reducing the overall piles of PR’s instead of whining yours isn’t being addressed?

I suspect you’ll change perspective after you’ve developed 15+ years and/or get some more personal hobbies that consume your personal time.

2 Likes

To whom are you refering to?

1 Like

My 2c, solution is not to review faster or more, solution is to have the HA platorm clean from integrations,
Have all integrations managed by HACS,
Have more strict rules to tegister custom components to HACS with automation tests (as part of pre-commit), and when there is a breaking change that requires opening code of many components it will be easy to notify the developers of that component as they are registered with their repo in HACS.

@isomer

Once your project gets approved, it’s pretty much instantaneous in HACS. There is no review process. Getting it approved is the bottleneck.

Of course that’s a double edged sword. Since no one reviews your code, as the project owner, it’s your responsibility to review it.

1 Like

Forcing moving to HACS is sweeping the issue under a carpet aka failure, it is not a solution. HACS grew into a somewhat close to must-have for HA. HACS also carries a lot of unusable stuff, however it isn’t relevant to the point. There’s enough integrations which are used instead of their core counterparts, sometimes, even because of the bugs and instabilities in the core ones. It’s effectively a claim that devs give no stability guarantee for integrations in HA.

Root cause from a 100 yards seems to be that Home Assistant is understaffed and underfunded, with maintainer priorities drifting away from users and an org structure that is struggling to scale to meet demand. Good engineers are not necessarily good managers. So basically typical big open source project issues. These are great problems to have, but need to be handled in the next few years or the project will be supplanted by another that can better handle the organization and funding side of things.

My suggested solution:

  • Break Home Assistant core into multiple repositories, identify components with most stale PRs, then assign more HA team members to regularly review those and/or grant top contributors to those repositories merge permissions.
  • Create strict PR merge requirements and enforce them by revoking permissions if a contributor is merging without verifying.
  • Have the HA team focus on building a rock solid integration testing framework so the volunteer reviewer army can scale without decreasing stability or putting all PRs on shoulders of HA team. Make adding integ tests (not just unit tests) part of PR merge requirements.

Not perfect, but better than current situation of asking contributors to review other contributor PRs they can’t merge, and PRs lingering for 6+ months as a result, often before being abandoned. Allot of community good will is being wasted.