Skip to content
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

Add Windows support. #712

Merged
merged 1 commit into from
Nov 7, 2019
Merged

Add Windows support. #712

merged 1 commit into from
Nov 7, 2019

Conversation

kolessios
Copy link
Contributor

This PR adds Windows support with the least possible number of changes, unlike other PRs with the same objective, this changes try to be friendly with the current code.

This is a summary of the most important changes:

  • The libraries cpy-cli and mkdirp were added to be alternatives to the cp and mkdir commands found in the package.json scripts. View
  • The cross-env library was added to execute the test command on all platforms.
  • The areas affected by path.join (orbit addresses) were changed to (path.posix || path). posix provides the correct way to join the strings and if it does not exist (in case of use on website) the normal path is used.
  • A static method called join has been created in the OrbitDBAddress class that allows to replace path.join for the purpose of the previous point. View
  • Windows has problems copying the LOG and LOCK files from tests/fixtures, a small filter has been created to avoid copying these files. View
  • Windows has trouble reading the files/binaries from tests/fixtures when they have been saved with EOL: CRLF, this causes 4 tests to fail, the solution is to prevent git from changing the EOL of the files. Source
  • 2 tests have been added to validate that an address with backslashes is considered valid. Source
  • ⚠ For some unknown reason the load v0 orbitdb address test fails most of the time (but not always) when it is first executed with the test:all command, if the test is run individually it passes without problems. I have called db.load() before each test to correct this problem, however it is not a real solution.
  • Other changes in the code are the result of running standard

  • All tests run smoothly in:
    • Windows 10 1903 (Build 18362)
    • Node v10.16.2

@dacom-dark-sun
Copy link

Excellent! Hope, it will be accepted by a core team.

@kolessios kolessios mentioned this pull request Nov 6, 2019
@aphelionz
Copy link
Member

Is there anything standing between us and merging this? @haadcode @shamb0t ?

@shamb0t
Copy link
Member

shamb0t commented Nov 7, 2019

Thanks a lot for the summary, @kolessios! Trying on a windows machine and the only failing test is the v0-backwards compatibility due to identity-migration failing, which is expected, so this looks good to merge 👍

@shamb0t shamb0t merged commit 186a46a into orbitdb:master Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants