-
Notifications
You must be signed in to change notification settings - Fork 585
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
[rush] Find rush.json location by using 'dirname' at most 10 times #4619
Comments
The way this code was merged into main is slightly weird (it was a cherry-pick from a That code likely can/should be refactored to just keep going until the disk root is reached. I might do that in the next few days, unless you, @tianmagongyue, are interested in doing that. |
OK, I can try to refactor this method with the idea 'keep going until the repo root is reached'. But it may take a little longer for me, and I need you or others to help me review the code. |
I was probably worried about the disk i/o cost in the negative case, for example if "rushx" is invoked repeatedly in a context where rush.json will never be found. Agree that this limit can be removed. |
Got it |
Btw there is a specific way to reliably detect when you reach the root on NTFS vs Unix filesystems. The PackageJsonLookup API in this repo is a good example to copy |
@tianmagongyue - did you create a fork of this repo and push your feature branch there? You shouldn't need any permissions to create a PR from a fork. |
ok, I'll create a fork |
I see rush try to find rush.json location by using 'Path.dirname' at most 10 times. Why was the final decision made ten times? It seems like a magic number. Why not 8 or 12 times?
The text was updated successfully, but these errors were encountered: