Skip to content
This repository has been archived by the owner on Apr 11, 2023. It is now read-only.

refactor: fix windows builds #347

Merged
merged 6 commits into from
Feb 8, 2022
Merged

refactor: fix windows builds #347

merged 6 commits into from
Feb 8, 2022

Conversation

amitojsingh366
Copy link
Contributor

@amitojsingh366 amitojsingh366 commented Feb 6, 2022

this PR fixes @shapeshiftoss/caip builds on windows
this PR should be merged after #346 as I did an oopsie (it contains changes from both PRs)
closes shapeshift/web#947

@amitojsingh366 amitojsingh366 requested a review from a team as a code owner February 6, 2022 01:21
@0xdef1cafe
Copy link
Collaborator

the fix here is not to change path naming conventions to something less correct, and update the docs to direct people to use WSL. we shouldn't be changing code to work with backwards file systems.

@amitojsingh366
Copy link
Contributor Author

the fix here is not to change path naming conventions to something less correct, and update the docs to direct people to use WSL. we shouldn't be changing code to work with backwards file systems.

but we won't be able to build the electron app for keepkey over CI then

@BitHighlander
Copy link
Contributor

BitHighlander commented Feb 7, 2022

if it's the goal of CAIP to become a standard lib in the space using special characters in the pathing is problematic. 74 percent of the world is on windows. using WSL might help a developer by using ubuntu under the hood, but that does not fix this CAIP package from throwing errors on any software it's bundled with running under windows. I understand that the change has 0 benefits for webs developer flow. and this is purely an aesthetics change of a : vr _. however this would greatly help us to not have to maintain a fork of the entire lib repo just to change a special character. and any other projects that are not web-based will be able to use this package and not worry about throwing errors after this modification.

@amitojsingh366
Copy link
Contributor Author

if it's the goal of CAIP to become a standard lib in the space using special characters in the pathing is problematic. 74 percent of the world is on windows. using WSL might help a developer by using ubuntu under the hood, but that does not fix this CAIP package from throwing errors on any software it's bundled with running under windows. I understand that the change has 0 benefits for webs developer flow. and this is purely an aesthetics change of a : vr _. however this would greatly help us to not have to maintain a fork of the entire lib repo just to change a special character. and any other projects that are not web-based will be able to use this package and not worry about throwing errors after this modification.

this was very properly phrased and I totally agree with this, @0xdef1cafe please give this another review

@0xdef1cafe
Copy link
Collaborator

@BitHighlander if you can confirm this is working locally after a yarn clean build and link into web i'll approve

@BitHighlander
Copy link
Contributor

@0xdef1cafe testing locally. working good :shipit:

@0xdef1cafe 0xdef1cafe merged commit f0ac218 into shapeshift:main Feb 8, 2022
@shapeshift-ci-bot
Copy link
Member

🎉 This PR is included in version @shapeshiftoss/caip-v1.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiled with problems
5 participants