fix: truncate docuhash return value in order to avoid ERRNAMETOOLONG error #4899
Conversation
|
Hi @tsirlucas! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
✔️ [V2] 🔨 Explore the source changes: 3fda0fc 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60c160ef77ccd60008271823 😎 Browse the preview: https://deploy-preview-4899--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4899--docusaurus-2.netlify.app/ |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
Got this e2e test error for some reason and updated my branch with master to check if it was the problem. Can you guys point me to a direction on why that was breaking? |
|
✔️ [V1] 🔨 Explore the source changes: 6bf1054 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-1/deploys/60c0d4c46c41e10007014de7 😎 Browse the preview: https://deploy-preview-4899--docusaurus-1.netlify.app |
Not sure why exactly but we have some random e2e failures sometimes, not a big deal. @tsirlucas unfortunately your test file does not seem to work on Windows and makes the CI fail, not sure how to fix this, as it seems windows complain about a too-long folder hierarchy on a git checkout Also, can you move all this code to a dedicated file. Ideally, if this code was stolen from another project, I'd rather link to the original source code (more convenient than using git history), try to isolate the code in a dedicated file and keep it like the original + also copy the tests. Maybe we could have |
|
@slorber Did the changes you asked for and added a possible fix for the windows problem. Unfortunately I couldnt test it locally because Also, I didnt add the test/logics for the |
|
thanks, LGTM 👍 |
closes #2628
Added a fix based on this. I did a local search and seems like everywhere that is using the
docuHashfunction, its related to a file path so I thought about just adding the code there. If you think its a better idea to add it to a different function and use this function instead ofdocuHashin the problematic places, just ping me.Motivation
#2628 (comment)
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Did some local testing with deep file paths and added unit test cases for the
docuHashfunction. Also, added a page for testing deep file paths to the website: http://localhost:3000/deep-file-path-test/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/foo/bar/test-file