Open tianmagongyue opened 5 months ago
The way this code was merged into main is slightly weird (it was a cherry-pick from a rush-next
branch about eight years ago), so I'm not finding any PR discussion around this. I'm pretty sure it's an arbitrary number that @octogonz picked with the thought that the CWD will probably never be >10 levels deeper than the repo root.
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.
The way this code was merged into main is slightly weird (it was a cherry-pick from a
rush-next
branch about eight years ago), so I'm not finding any PR discussion around this. I'm pretty sure it's an arbitrary number that @octogonz picked with the thought that the CWD will probably never be >10 levels deeper than the repo root.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.
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
hi, I have refactored the tryFindRushJsonLocation function, and I need to be a collaborator of rushstack so that I can start a merge request. cc @iclanton @octogonz
@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.
@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?