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.

Sign up for GitHub

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

Jump to bottom

New build system of Hass.io #3406

Merged
merged 18 commits into from
Sep 26, 2017
Merged

New build system of Hass.io #3406

merged 18 commits into from
Sep 26, 2017

Conversation

pvizeli
Copy link
Member

@pvizeli pvizeli commented Sep 19, 2017 •
edited

Description:

  • updates all docs
  • update core addons
  • add blogpost

@pvizeli pvizeli added blog This PR adds a blog article to the Home Assistant website Enhancement new-feature This PR adds documentation for a new Home Assistant feature to an existing integration labels Sep 19, 2017
@pvizeli pvizeli changed the title Update addon_config.markdown New build system of Hass.io Sep 19, 2017
@frenck
Copy link
Member

frenck commented Sep 19, 2017

I do not totally agree on the chosen path and implementation. The parent PR has been merged, ignoring my comment about having concerns.

I therefore cannot support this at this point.

@MartinHjelmare
Copy link
Member

What are the concerns? I'm interested. Maybe link to the PR and add them there, or in a separate issue at hassio?

@pvizeli pvizeli changed the title New build system of Hass.io WIP: New build system of Hass.io Sep 19, 2017
@frenck
Copy link
Member

frenck commented Sep 20, 2017

@MartinHjelmare I will provide some more explanation asap.

@frenck
Copy link
Member

frenck commented Sep 20, 2017

@MartinHjelmare This is the related pull request: home-assistant/supervisor#191

@frenck
Copy link
Member

frenck commented Sep 20, 2017

I've opened an GitHub issue in order to get a discussion started.

home-assistant/supervisor#198

@frenck frenck mentioned this pull request Sep 20, 2017
Closed
@frenck
Copy link
Member

frenck commented Sep 23, 2017

The discussion on my concerns has been closed.
So I no longer see anything that is holding this PR back from my end.

This was referenced Sep 23, 2017
Merged
Merged
@pvizeli pvizeli changed the title WIP: New build system of Hass.io New build system of Hass.io Sep 25, 2017
@pvizeli pvizeli added the ready-for-review This PR needs to be reviewed label Sep 25, 2017
@frenck frenck self-requested a review September 25, 2017 20:33
frenck
frenck reviewed Sep 25, 2017
View reviewed changes
source/developers/hassio/addon_config.markdown Outdated
@@ -135,7 +138,9 @@ The `options` dict contains all available options and their default value. Set t
{ "username": "cheep", "password": "654321" }
],
"random": ["haha", "hihi", "huhu", "hghg"],
"link": "http://blebla.com/"
"link": "http://blebla.com/",
Copy link
Member

Choose a reason for hiding this comment

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

Please use example.com for documentation purposes. This is the official domain reserved for this by the IANA and described in RFC 2606.

@frenck
Copy link
Member

frenck commented Sep 25, 2017

I've added some quick fixes to the developer documentation part.
The blog post still needs some ❤️ .

@pvizeli
Copy link
Member Author

pvizeli commented Sep 26, 2017

Perfect

@balloob balloob merged commit a1a4ff2 into current Sep 26, 2017
@balloob balloob deleted the hassio-new-build-system branch September 26, 2017 06:51
@frenck
Copy link
Member

frenck commented Sep 26, 2017

Thanks for the mention in the blog post 👍
Good job guys 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frenck frenck frenck left review comments

Assignees

@pvizeli pvizeli

Labels
blog This PR adds a blog article to the Home Assistant website new-feature This PR adds documentation for a new Home Assistant feature to an existing integration ready-for-review This PR needs to be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pvizeli @frenck @MartinHjelmare @balloob