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

This WTH is more about the development process, is that also allowed?

Beginning of March I opened a PR on Github, it took 2 months before someone reviewed it, a month later I got some comments from a core developer bus since then it was not really reviewed by a core developer again. The only comment I got from a core developer what that I didn’t use a feature that was added a few weeks before. Sure I didn’t, the PR was from long before that.

So, either the feature is not valued or there’s a large backlog of PRs to review, or maybe both. In either way it’s discouraging to contribute.

TL;DR What the heck, are contributions actually wanted? Or only for popular features/bugfixes?

It highly depends on the context. No 2 PRs are alike. I’ve had PRs take a week, I’ve had PRs take 6 months. It also depends on how active the code owner is. The main devs don’t need to review everything. Any review is good from a member.

1 Like

I think they’re overloaded and have to prioritise as best they can. It won’t be perfect.

If they’re overloaded, then joining Hacktoberfest might not be such a good idea? It’ll result in a lot more PRs. Also, being overloaded can be partially solved by better defining certain preferred styles of programming, exceptions you want to have thrown, preference for explicit checks or catching exceptions etc. Just some taste depending things. This allows for any contributor to better review someone else’s work and could result in easier reviews for the core devs.

I know that reviewing costs a lot of time and that prioritising is needed. I only have the feeling that newer PRs get more priority and that once your PR was missed when it was new, it can take forever to be closed. Which is maybe logical because it seems most of the older PRs are not really updated anymore. Still, it would be nice if there’s a way of indicating that a PR can be reviewed and is still actively worked on.

3 Likes

THey are all actively getting looked at. If you want it to appear in what you’re referring to as the ‘new’ list. Simply reply in the PR, that keeps it towards the top. If you let it sit dormant, it floats to the bottom.

Maybe the supervision process could be improved somewhat. A possibility could be to have the bot scan items that have been open for a long time and send a reminder to the code owner(s).

However, some issues are not addressed by a bot as well, such as my PR for a problem in the SMTP mail.

I’ve got a PR up now too. I took @petro 's advice and added a new comment:

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