3 ms·

Having contributed to a number of projects via github, this is not true at all. I have had prs closed within minutes with curt replies. That is not how you “hel

by ghroos 9y ago

Having contributed to a number of projects via github, this is not true at all. I have had prs closed within minutes with curt replies. That is not how you “help newer devs through a PR process.” In another case this happened for a very clear bug. I’m cool with making amendments but maintainers do not seem open to that often, feeling more content to close things because they see the fix as a “hack”. Compared to other projects HASS is one of the worst I have dealt with, sorry to say.

m4x 9y ago
This has not been my experience at all when contributing to HA. Maybe we interacted with different people. They do have strict code standards, but it's hard to argue that's a bad thing.
- ghroos 9y ago
  Probably. I had a couple prs accepted by another maintainer fwiw. The code standards were not the issue, nor were unit tests. Not my first rodeo. The later interactions were not pleasant and did not engender much interest in further contribution.
xur17 9y ago
As a counterpoint, I had a really good experience with the 2 or 3 PRs I submitted. They were merged pretty quickly with helpful feedback. It definitely makes me more likely to contribute in the future.
JshWright 9y ago
Obviously this is an entirely subjective thing, and neither of us is "right" or "wrong". Home Assistant has certainly gotten a lot more particular about new code as the project has grown, but I still feel they do a great job of helping new users along. Bummer you had such a negative experience...
- ghroos 9y ago
  Which is sad considering an issue I was attempting to contribute was a bug fix rather than new feature work. Clearly a lot of the earlier code suffers from lack of design. As I mentioned later I have had PRs accepted but overall I find maintainership to be a real mixed bag. Better run open source projects do not have this inconsistency.
SEJeff 9y ago
Mind mentioning a link to the PRs you're referring to?
- ghroos 9y ago
  Sure. https://github.com/home-assistant/home-assistant/pull/9272 https://github.com/home-assistant/home-assistant/pull/9272 Closed with prejudice as a “hack.” I actually spent some time to make this feature request implementation consistent with other code as another contributor mentioned. https://github.com/home-assistant/home-assistant/pull/9270 https://github.com/home-assistant/home-assistant/pull/9270 The second one was based on an observed defect in the UI. The defect is still there but apparently this is flippantly a “feature” now. Disagreements about implementation aside this is not the response I’ve gotten from better maintained projects.
  - balloob 9y ago
    Sorry to hear that you had a bad experience. As an open source project we get bombarded with pull requests where people have spent time and research to create something without reaching out to us in advance. This means that sometimes the way you are architecting your new feature doesn't fit with the current design or our vision. It means that we will sometimes say the feature needs changing. We are all volunteers and the last thing we want is to start argueing with every person on the internet. The maintainers would burn out pretty quickly. And that means that if we are looking at 20 PRs a night, we will make mistakes and judge too quickly. Can we look at less PRs and spent more time on each PR? Sure, then people get pissed that it takes too long to get feedback on their PRs. There is no winning for us here and so each maintainer will have to find their balance. For 9272, Pascal has offered the correct solution: a service switch. Every platform could be sharing code with other platforms given enough configuration parameters. We have made the decision that that is not in our best interest as it means that growing one platform now has to be made compatible with all the other configuration options and it becomes a mess. So create a new platform if your change is changing the core of a platform (like removing the template from a template switch). Your comments after the PR got closed got probably missed. For 9270, you rebased after the discussion took place and I don't remember if the code was different before. The current change looks fine for me. You've removed your clone so I can't re-open the PR for you, but feel free to open a new PR with those changes and tag me in the PR message.

by ghroos 9y ago

m4x 9y ago

This has not been my experience at all when contributing to HA. Maybe we interacted with different people. They do have strict code standards, but it's hard to argue that's a bad thing.

ghroos 9y ago
Probably. I had a couple prs accepted by another maintainer fwiw. The code standards were not the issue, nor were unit tests. Not my first rodeo. The later interactions were not pleasant and did not engender much interest in further contribution.

xur17 9y ago

As a counterpoint, I had a really good experience with the 2 or 3 PRs I submitted. They were merged pretty quickly with helpful feedback. It definitely makes me more likely to contribute in the future.

JshWright 9y ago

Obviously this is an entirely subjective thing, and neither of us is "right" or "wrong". Home Assistant has certainly gotten a lot more particular about new code as the project has grown, but I still feel they do a great job of helping new users along. Bummer you had such a negative experience...

ghroos 9y ago
Which is sad considering an issue I was attempting to contribute was a bug fix rather than new feature work. Clearly a lot of the earlier code suffers from lack of design. As I mentioned later I have had PRs accepted but overall I find maintainership to be a real mixed bag. Better run open source projects do not have this inconsistency.

SEJeff 9y ago

Mind mentioning a link to the PRs you're referring to?

ghroos 9y ago
Sure. https://github.com/home-assistant/home-assistant/pull/9272 https://github.com/home-assistant/home-assistant/pull/9272 Closed with prejudice as a “hack.” I actually spent some time to make this feature request implementation consistent with other code as another contributor mentioned. https://github.com/home-assistant/home-assistant/pull/9270 https://github.com/home-assistant/home-assistant/pull/9270 The second one was based on an observed defect in the UI. The defect is still there but apparently this is flippantly a “feature” now. Disagreements about implementation aside this is not the response I’ve gotten from better maintained projects.
- balloob 9y ago
  Sorry to hear that you had a bad experience. As an open source project we get bombarded with pull requests where people have spent time and research to create something without reaching out to us in advance. This means that sometimes the way you are architecting your new feature doesn't fit with the current design or our vision. It means that we will sometimes say the feature needs changing. We are all volunteers and the last thing we want is to start argueing with every person on the internet. The maintainers would burn out pretty quickly. And that means that if we are looking at 20 PRs a night, we will make mistakes and judge too quickly. Can we look at less PRs and spent more time on each PR? Sure, then people get pissed that it takes too long to get feedback on their PRs. There is no winning for us here and so each maintainer will have to find their balance. For 9272, Pascal has offered the correct solution: a service switch. Every platform could be sharing code with other platforms given enough configuration parameters. We have made the decision that that is not in our best interest as it means that growing one platform now has to be made compatible with all the other configuration options and it becomes a mess. So create a new platform if your change is changing the core of a platform (like removing the template from a template switch). Your comments after the PR got closed got probably missed. For 9270, you rebased after the discussion took place and I don't remember if the code was different before. The current change looks fine for me. You've removed your clone so I can't re-open the PR for you, but feel free to open a new PR with those changes and tag me in the PR message.