-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
feat: find .tool-versions
and legacy files until reach root directory and keep .tool-versions
order.
#289
base: main
Are you sure you want to change the base?
Conversation
It seems that there is no sorting implementation in |
Yes, only ensures that the order of adding. |
76d2508
to
4da6bab
Compare
flushCache.Clear() | ||
} | ||
if item.Version == version { | ||
logger.Debugf("Hit cache, skip flush envrionment, %s@%s\n", name, version) |
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.
typo
} | ||
wg.Wait() | ||
} | ||
if versionMap.Len() != 0 { |
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.
I may not understand the code correctly. As soon as the first .toolsversion is found, the recursion is stopped. However, it could be that not all SDKs are specified in this file. In other words, I would expect a combination of all .toolsversions from all paths. The global file has the lowest priority and then ascending according to proximity to the current working directory.
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.
You reminded me that the global .tool-versions
file should be combined.
In other words, I would expect a combination of all .toolsversions from all paths
I don't agree to this point.
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.
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.
Sorry for the late reply. I have got it, and I will continue to improve this PR soon later.
fix #269
fix #228
fix #288