Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move imports in wake_on_lan component #28100

Merged
merged 2 commits into from
Oct 23, 2019
Merged

Move imports in wake_on_lan component #28100

merged 2 commits into from
Oct 23, 2019

Conversation

djpremier
Copy link
Contributor

@djpremier djpremier commented Oct 22, 2019

Description:

Moved imports from functions to top-level as requested in #27284

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

@fabaff
Copy link
Member

fabaff commented Oct 22, 2019

Duplicate of #27415

@fabaff fabaff marked this as a duplicate of #27415 Oct 22, 2019
@djpremier djpremier marked this pull request as ready for review October 23, 2019 02:03
@djpremier
Copy link
Contributor Author

@fabaff, thanks for returning. But I know that this pull is duplicated, I did it to correct the tests, since @inverse didn't give any answer already 12 days ago

@balloob balloob merged commit b4054ad into home-assistant:dev Oct 23, 2019
@djpremier djpremier deleted the wake_on_lan_fix_imports branch October 23, 2019 12:32
@inverse
Copy link
Contributor

inverse commented Oct 23, 2019

@djpremier it was 5 days not 12 :/

@balloob - is there a policy on the time for dupes?

@djpremier
Copy link
Contributor Author

djpremier commented Oct 23, 2019

Hi @inverse, I believe there is no time policy, I commented there because it is the information that is appearing to me here on Git (https://ibb.co/D42Z0HQ). As you did not respond, I decided to make the changes so that the issue could be closed as soon as possible, we are an Open Source community, we are here to help, sorry if I offended you in any way, it was not my intention.

@inverse
Copy link
Contributor

inverse commented Oct 24, 2019

@djpremier I think there should be some policies around "trumping" PR's etc since that could demotivate people from contributing which would have an much more negative effect on an Open Source community than closing issues "as soon as possible".

@frenck
Copy link
Member

frenck commented Oct 24, 2019

@inverse We don't have a policy for that, nor do I personally think that is needed.

I'm sorry to hear that this has happened, however, this PR did pass the CI, the other didn't. It makes sense to merge the passing contribution. Closing it by saying: "Nope, great PR, but there is a dupe that needs to be fixed first" is awkward as well.

In the end, what matters is that we move forward, so everybody can enjoy this great piece of software. This is not a competition. We value every contributor and contribution. Merged or not merged doesn't matter.

So let me close by saying; Thanks @inverse, please don't be sad and I would definitely hope to see a contribution from you sometime again. ❤️

@lock lock bot locked and limited conversation to collaborators Oct 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants