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.

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

fix(tools/check_for_upgrade): Don't source profile #2431

Closed
wants to merge 1 commit into from

Conversation

feltnerm
Copy link
Contributor

Reverts #2296, but mostly #1883.

There is no need to source ~/.profile when this script is read. oh-my-zsh writes no configuration data in ~/.profile.

If the user wishes to use data within ~/.profile, then they should source it in another place.

Fixes #2315
Fixes #1509
Fixes #1898

Reverts ohmyzsh#2296, but mostly ohmyzsh#1883. 

There is no need to source ~/.profile when this script is read. oh-my-zsh writes no configuration data in ~/.profile.

If the user wishes to use data within ~/.profile, then they should source it in another place.

Fixes ohmyzsh#2315
@mcornella
Copy link
Member

I support this, issues #1509 and #1898 have failed because of that. Granted, those people's configurations are weird and mistakes in general. But it is not necessary and can generate problems if it's executed via oh-my-zsh autoupdate..
What do you say @stibbons (from #1883), can you manage to setup your proxy config any other way?

@mcornella
Copy link
Member

Related issue #2315 and maybe #2428. What do you say @ncanceill?

@feltnerm
Copy link
Contributor Author

👍

@mcornella, I still kinda can't believe that #1883 was merged in in the first place...

@ncanceill
Copy link
Contributor

I agree, ~/.profile should be sourced directly by the user, probably in their ~/.zshrc. This should mitigate many configuration errors, because many OMZ users switched from bash/ksh to zsh at the same time they installed OMZ.

@mcornella
Copy link
Member

Hi @feltnerm, issues #1509 and #1898 are both happening because of sourcing .profile, so this PR will fix them. Can you add this to your PR description?

Fixes #1509
Fixes #1898

@feltnerm
Copy link
Contributor Author

@mcornella -- what's the best way for me to do that? Will just adding them to the description automatically close them, or do they need to be in the actual commit message?

@mcornella
Copy link
Member

Yes, adding to the description of a PR will close them. I made some experiments regarding that, so I know what I'm talking about 😁

@feltnerm
Copy link
Contributor Author

Awesome, good work! Updated the description.

@mcornella
Copy link
Member

Cool thanks!

@feltnerm
Copy link
Contributor Author

Yay 😀 thanks for your hard work everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants