-
Notifications
You must be signed in to change notification settings - Fork 18
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
Migrate 'cmd' to 'main', licence update #97
Conversation
* All command-line functionality now in 'main' package * 'inertia daemon' commands are now completely disabled outside of the inertia-daemon image
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.
Changes look straightforward!
Should we also update the year in the LICENSE file?
@@ -33,7 +33,7 @@ sudo docker run -d --rm \ | |||
-p "$PORT":8081 \ | |||
-v /var/run/docker.sock:/var/run/docker.sock \ | |||
-v $HOME:/app/host \ | |||
-e DAEMON="true" \ | |||
-e INERTIA_DAEMON='true' \ |
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.
Just an observation - it looks like use of " " and ' ' is inconsistent in our shell scripts.
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.
That's a good point... I'm not actually sure how interchangeable they are 🤔
Will look into it
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 think the way we have it now is fine: see this explanation for single vs double quotes in bash
Single for string literals, double for things like "$MYVAR"
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.
Cool, thanks for the link!
Thanks for the review @PiggySpeed! |
Thanks for the review @rogermyang! This one will affect your ticket (#72), remember to make sure you are up to date with |
🎟️ Ticket(s):
👷 Changes
This is largely a simple code migration. ⏫
daemon
and having it conflict with the command 😢Apache 2.0
licence - we are already underMIT
🔦 Testing Instructions
go install inertia daemon run # should no longer work