-
Notifications
You must be signed in to change notification settings - Fork 83
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
Check for updates when starting a new shell. #153
base: master
Are you sure you want to change the base?
Conversation
We removed Can you make sure that we can auto-update with |
But there's a following commit ca7839a, which prevents So I guess it's safe to use |
I think this is fine except I'm not sure if we need to call IMHO, the interface should be like this so that there are no surprises:
But we don't have to change this. |
Before c27b213, |
@@ -26,7 +26,7 @@ def inject_parts_init(path) | |||
file = File.read(path) | |||
File.open(path, 'a') do |f| | |||
export_path = "export PATH=\"$HOME/#{relative_autoparts_bin_path}:$PATH\"\n" | |||
parts_init = "eval \"$(parts env)\"\n" | |||
parts_init = "eval \"$(parts init -)\"\n" |
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.
Because parts_init
var is changed here, line 32 file.include? parts_init
condition will mean that the file can potentially have two lines, one for evalling parts env
the other for evalling parts init
.
Which I guess is fine, but something to watch out for in the future if we change it back.
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.
hmmm..but I guess it would be rare for someone to run the setup script, if they have Autoparts already installed?
This reinstates the original behaviour of checking for updates (which is included in
parts init
) when starting a new shell.