Skip to content

Add documentation for Unifi LED integration#10720

Merged
frenck merged 6 commits into
home-assistant:nextfrom
florisvdk:patch-2
Nov 6, 2019
Merged

Add documentation for Unifi LED integration#10720
frenck merged 6 commits into
home-assistant:nextfrom
florisvdk:patch-2

Conversation

@florisvdk
Copy link
Copy Markdown
Contributor

@florisvdk florisvdk commented Oct 11, 2019

Description:
Added unifiled integration documentation in preparation of making a pull request for it.

Pull request in home-assistant (if applicable): home-assistant/core#27475

Checklist:

  • Branch: next is for changes and new documentation that will go public with the next Home Assistant release. Fixes, changes and adjustments for the current release should be created against current.
  • The documentation follows the standards.

@probot-home-assistant probot-home-assistant Bot added current This PR goes into the current branch Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! labels Oct 11, 2019
@florisvdk florisvdk mentioned this pull request Oct 11, 2019
9 tasks
@probot-home-assistant probot-home-assistant Bot added the has-parent This PR has a parent PR in another repo label Oct 11, 2019
Copy link
Copy Markdown
Member

@klaasnicolaas klaasnicolaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left som comments, btw please rebase this PR to the next branch.

Comment thread source/_integrations/unifiled.markdown
Comment thread source/_integrations/unifiled.markdown Outdated
Comment thread source/_integrations/unifiled.markdown Outdated
Comment thread source/_integrations/unifiled.markdown Outdated
Comment thread source/_integrations/unifiled.markdown
@klaasnicolaas klaasnicolaas added in-progress This PR/Issue is currently being worked on needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch new-integration This PR adds documentation for a new Home Assistant integration labels Oct 11, 2019
@florisvdk florisvdk changed the base branch from current to next October 11, 2019 18:50
@probot-home-assistant probot-home-assistant Bot added next This PR goes into the next branch and removed current This PR goes into the current branch labels Oct 11, 2019
@florisvdk
Copy link
Copy Markdown
Contributor Author

florisvdk commented Oct 11, 2019

I think i messed something up with the rebasing. I am new to a lot of this.

Is this fixable or do i make a new pull request?

@klaasnicolaas
Copy link
Copy Markdown
Member

Now you only chaged the target branch, but you must also rebase the patch-2 branch of your fork.

Fixed marked issues

The option "Id" is going to be removed in the integration, making a commit for that after this.

Added . to end of line
@florisvdk
Copy link
Copy Markdown
Contributor Author

Finally figured it out, Thanks for putting up with this.

Comment thread source/_integrations/unifiled.markdown Outdated
@klaasnicolaas
Copy link
Copy Markdown
Member

Good to see that you have succeeded in solving the problem, with a few small comments left we can approve it 😉

Co-Authored-By: Klaas Schoute <klaas_schoute@hotmail.com>
@florisvdk
Copy link
Copy Markdown
Contributor Author

Made a new commit to fix the settings in the integration code.

@klaasnicolaas klaasnicolaas removed in-progress This PR/Issue is currently being worked on needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch labels Oct 12, 2019
Floris Van der krieken and others added 2 commits October 12, 2019 16:29
Co-Authored-By: Klaas Schoute <klaas_schoute@hotmail.com>
@florisvdk
Copy link
Copy Markdown
Contributor Author

@klaasnicolaas I fixed the things you asked for.

klaasnicolaas
klaasnicolaas previously approved these changes Oct 19, 2019
@frenck frenck added the awaits-parent Awaits the merge of an parent PR label Oct 20, 2019
Comment thread source/_integrations/unifiled.markdown Outdated
required: true
default: None
port:
description: Port used to connect to the Unifi LED controller.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to make this configurable at all? Other than that, description such as Control Port, Login username would be shorter and easier to read.

Comment thread source/_integrations/unifiled.markdown Outdated
Comment thread source/_integrations/unifiled.markdown Outdated
Comment thread source/_integrations/unifiled.markdown
Comment thread source/_integrations/unifiled.markdown Outdated
@probot-home-assistant probot-home-assistant Bot added the parent-merged The parent PR has been merged already label Oct 24, 2019
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @florisvdk! 👍 Sorry for the long wait! 🙏

@frenck frenck merged commit 4b997e3 into home-assistant:next Nov 6, 2019
@probot-home-assistant probot-home-assistant Bot removed awaits-parent Awaits the merge of an parent PR parent-merged The parent PR has been merged already labels Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! has-parent This PR has a parent PR in another repo new-integration This PR adds documentation for a new Home Assistant integration next This PR goes into the next branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants