Skip to content
This repository was archived by the owner on Oct 30, 2019. It is now read-only.

Moved user input to the top of the scripts.#109

Merged
Landrash merged 3 commits into
home-assistant:devfrom
ludeeus:move-all-userinput
Feb 12, 2018
Merged

Moved user input to the top of the scripts.#109
Landrash merged 3 commits into
home-assistant:devfrom
ludeeus:move-all-userinput

Conversation

@ludeeus
Copy link
Copy Markdown
Member

@ludeeus ludeeus commented Feb 12, 2018

Description:

  • Moved all user input to the top of the scripts.
    Cosmetic change, so the user can go get a cup of coffee/tea while the script run, and not worry about it hanging waiting for input.
  • Where there was an validation this line is not needed an has been removed:
    "If you have issues with this script, please say something in the #devs_hassbian channel on Discord."
    This line will still print if the validation fail.
  • Added validation to appdaemon-install-package

Related issue (if applicable): Fixes #108

Checklist:

  • The code change is tested and works locally.
  • Script has validation check of the job.
    • Not really applicable here.

If pertinent:

  • Created/Updated documentation at /docs
    • Not really applicable here.

@ludeeus ludeeus added this to the v0.8.0 milestone Feb 12, 2018
Copy link
Copy Markdown
Collaborator

@Landrash Landrash left a comment

Choose a reason for hiding this comment

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

Some spelling and formatting will need to be looked at but looks good in general.


if [ "$ACCEPT" != "true" ]; then
if [ -f "/usr/sbin/samba" ]; then
echo -n "Do you want to add samba share for AppDaemon configuration? [N/y] : "
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Samba with capital S

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👍

echo
echo -e "\e[32mInstallation done..\e[0m"
echo
echo "You will find the appdaemon configuration files in:"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Formatting white space missing here and two line more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure how :/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's a "TAB" that's missing before the echo

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I see that is missing, not sure how that happened :)

echo
echo -e "\e[32mInstallation done..\e[0m"
echo
echo "You will find the appdaemon configuration files in:"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Appdaemon with Capital A?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Capital D to :)

echo "Preparing system, and adding dependencies..."
sudo apt update
sudo apt -y upgrade
sudo apt install -y make git
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't git and make both installed in the base image?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They sure are :)


if [ "$ACCEPT" != "true" ]; then
if [ -f "/usr/sbin/samba" ]; then
echo -n "Do you want to add samba share for Homebridge configuration? [N/y] : "
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Capital S for Samba.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👍

sudo apt install -y nodejs
sudo apt install -y libavahi-compat-libdnssd-dev

echo "Installing homebridge for homeassistant..."
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Capital H for Homebridge, captial for Home Assistant and spelling for Home Assistant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👍

fi
fi
if [ "$SAMBA" == "y" ] || [ "$SAMBA" == "Y" ]; then
echo "Adding configuration to samba..."
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Capital S in Samba

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👍

echo
echo "If you have issues with this script, please say something in the #devs_hassbian channel on Discord."
echo
echo
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Formatting white space missing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👍

@ludeeus
Copy link
Copy Markdown
Member Author

ludeeus commented Feb 12, 2018

Found some more spelling errors :)

echo
echo "You will find the appdaemon configuration files in:"
echo "You will find the AppDaemon configuration files in:"
echo "/home/homeassistant/appdaemon"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Missing white space.

echo
echo "You will find the appdaemon configuration files in:"
echo "You will find the AppDaemon configuration files in:"
echo "/home/homeassistant/appdaemon"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Missing white space.

Copy link
Copy Markdown
Collaborator

@Landrash Landrash left a comment

Choose a reason for hiding this comment

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

Looks good!
Great work as usual 🍰

@Landrash Landrash merged commit c7aabc6 into home-assistant:dev Feb 12, 2018
@ludeeus ludeeus deleted the move-all-userinput branch February 13, 2018 16:54
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.

3 participants