-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add installation file for ubuntu and osx #524
base: main
Are you sure you want to change the base?
Conversation
works on my machine (: |
Thanks for this @davidmanzanoai ! I've got some general questions about the approach, more than the technical implementation. If we're trying to have a way for people to install lumigator easily, I think this tool shouldn't clone the repository at all (as it's not needed to run lumigator!) My favored approach here would be to have a script like this one, that pulls the docker-compose & run the start-lumigator command, we probably would like to have a specific docker-compose as well for that one with pre-initiated variables. What are your thoughts? |
@agpituk What we can do is to set up the script in a new folder (e.g. install) and we can use the link to the script to be used for installing everything. The default choice will be to take the zip file and use the package from the package repositories instead of building locally (that option will remain accesible through out the Makefile). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have tried it on WSL and it works. I'd propose enhancing this to just retrieve the docker compose files and let it download the necessary images from the image repos.
Ok, happy with that path. We'd need to update the docs as well |
@agpituk @javiermtorres @ividal Could you please take a look. In OS X, if docker isn't installed, it stoped and points the user to a manual installation. Otherwise it sets up everything for the user and start the browser. We might add the file into a folder if you prefer |
# Download based on method | ||
|
||
echo "Downloading ZIP file..." | ||
curl -L "${REPO_URL}/archive/main.zip" -o lumigator.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be a release link at some point. Maybe just after MVP, when we do have a tag and a release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't check it this time, but left some comments. Just check that it really works :)
Co-authored-by: Kostis <[email protected]> Signed-off-by: David Manzano Macho <[email protected]>
Co-authored-by: Kostis <[email protected]> Signed-off-by: David Manzano Macho <[email protected]>
11387a6
to
7ef0267
Compare
7ef0267
to
36424e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty handy and worked well on both OS and Ubuntu, except for the docker-compose detection which did not find it on either.
if ! command -v docker &> /dev/null | ||
then | ||
echo "Docker not found. Installing Docker..." | ||
sudo apt-get update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh. Needing sudo rights. Many devs at companies will have docker available to them through their IT but very likely not have sudo rights to the machine they're using.
Not necessary to merge, but we could have a top level script that asks/checks and then calls:
- (optional) one that requires sudo and takes care of pre-requirements like docker.
- a second one with everything else, which assumes you have docker correctly set up for you.
|
||
# Install Docker Compose | ||
install_docker_compose() { | ||
if ! command -v "$BREW_PATH/docker-compose" &> /dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I did have docker compose installed this still didn't detect it.
What's changing
Provide a clear and concise description of the content changes you're proposing. List all the
changes you are making to the content.
The script allows in a easy way to set up the local environment for starting the development or local usage of the tool. It secures that docker, docker-compose and even git is installed or otherwise do it for you. Once downloaded the script, it requires to add chmod +x install-lumigator.sh. After, you decide how to execute it:
Refs #...
Closes #...
How to test it
Steps to test the changes:
Additional notes for reviewers
Anything you'd like to add to help the reviewer understand the changes you're proposing.
I already...
/docs
)