-
Notifications
You must be signed in to change notification settings - Fork 177
feat: Add Updated Setup For Windows To Readme #3703
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: aceppaluni <aceppaluni@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
README.md
Outdated
| ### Windows | ||
| Enable the long paths in Windows: | ||
|
|
||
| 1. Go to ```Run``` app and type ```regedit``` | ||
| 2. Navigate to ```HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem``` | ||
| 3. On the right search for ```LongPathsEnabled``` entry | ||
| 4. Set this to ```Enabled``` or ```1``` | ||
| 5. Open Git Bash as an Administrator and run ```git config --system core.longpaths true``` | ||
| 6. In Git Bash Run ```git config core.autocrlf false``` | ||
| 7. FInally, in Git Bash run ```git config core.eol lf``` |
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.
Thanks for putting this together - I know I initially suggested adding more explicit Windows guidance, but on second thought I think we may want to slightly adjust the approach.
Instead of documenting the exact registry and Git configuration steps, it might be better to describe the required conditions at a higher level (e.g. long paths must be enabled on Windows, Git must support long paths, LF line endings enforced), and link to official documentation for how to configure those.
My thinking now is that these are general OS/Git configuration topics rather than SDK-specific setup, and keeping the README focused on SDK requirements will likely make it cleaner and easier to maintain long term.
Curious what you think about reframing it that way.
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.
Also with this in place, I do not think that we should split the README.md per OS as these would be +2~lines of text. :)
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Signed-off-by: aceppaluni <aceppaluni@gmail.com>
Signed-off-by: aceppaluni <aceppaluni@gmail.com>
Signed-off-by: aceppaluni <aceppaluni@gmail.com>
Signed-off-by: aceppaluni <aceppaluni@gmail.com>
README.md
Outdated
| ### Windows | ||
|
|
||
| >[!Note] | ||
| > Link to official documentation: [Windows-Long-Path](https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry) |
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.
| > Link to official documentation: [Windows-Long-Path](https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry) | |
| > Long paths must be enabled. Link to official documentation: [Windows-Long-Path](https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry) |
README.md
Outdated
| > | ||
| > Git must support long paths | ||
| > | ||
| > LF line endings must be enforced on Windows |
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.
| > LF line endings must be enforced on Windows | |
| > LF line endings must be enforced. |
Since it is obvious it is for Windows as we're in the windows section
Signed-off-by: aceppaluni <aceppaluni@gmail.com>
venilinvasilev
left a comment
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.
LGTM
Description:
This PR aims to update the readme with new setup instructions for Winodows users to setup the JS SDK on their local Winodws machine.
Related issue(s):
Fixes #3697
Notes for reviewer:
Attempted to add a clear defined step for Windows users only
Checklist