-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
lib: fail gracefully if we can't find the username #2375
lib: fail gracefully if we can't find the username #2375
Conversation
Don't run this outside of a try block, as it can fail in some scenarios, especially some containerization or virtual system scenarios with no user ID or username.
Notes for reviewers: Logging: I hoped to add logging to this, but I couldn't think of a way to do that without adding a new logging function at the file-level scope (outside of
Stop trying to use |
@DeeDeeG hi. As i see, you are working on |
I believe this PR applies cleanly over your utf-8 branch, and vice-versa; There should be no merge conflicts here. (I did make a branch with a lot of refactoring and logging, but I actually think that much change is inappropriate for a bug fix PR. And I am not pursuing that much refactoring until some of the backlog of open, recent PRs is addressed some.) Yes, coordinating is a good idea if multiple people are working on the same file in parallel. But I am not planning to work on this repo for now, until some open pull requests get merged or closed. I don't think there is a point to contribute with a backlog of open PRs that might land, because it does indeed make it harder to predict which ones will conflict. And because it has actually been over a month since anything was landed at this repository, I am not sure when/if PRs will be merged. I don't want to propose a lot of changes, and potentially have to remember what I was working on months later to address feedback from reviewers. So for now I am not planning on submitting any additional PRs, until the backlog of recent open PRs is addressed. So I hope that means you don't have to worry about conflicts from my branches. Best regards. P.S. I understand most Node folks are volunteers and have a lot of various responsibilities. This is in no way a complaint or critique. I just want to put this out in addressing @owl-from-hogvarts's suggestion of coordination, and avoiding conflict between PRs. I think as I'm not posting new PRs it shouldn't be an issue. And it's a moot point until there is the real chance things start getting merged again. P.P.S. I am sorry that my other PR #2333 caused merge conflicts for your UTF-8 PR. |
Wow! thanks for so detailed answer 😄 |
Checklist
npm install && npm test
passesDescription of change
In
lib\find-python.js
...os.userInfo().username
in atry {}
block, since it's not 100% guaranteed to succeedAppdata\Local
directory, skip checking paths underAppdata\Local
.Fixes #2373