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

docs(rtd): update windows instructions #718

Merged

Conversation

wallw-teal
Copy link
Contributor

Updates our Windows development documentation by moving it to the RTD docs and updating it per ngageoint/opensphere-build-resolver#46.

Copy link
Collaborator

@justin-bits justin-bits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. git config --global core.autocrlf input needs to be included in the documentation too. Without this, the build fails with 277k+ errors Expected linebreaks to be 'LF' but found 'CRLF' linebreak-style.
  2. I'm not seeing commitizen install mentioned, should that be here too?

docs/windows_development.rst Outdated Show resolved Hide resolved

npm run build

At the time of this writing, ``yarn`` has a bug with the ``script-shell`` config. A fix has already been committed and should make their next non-patch release. After that you can use ``yarn run build`` or just ``yarn build`` if you prefer.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
At the time of this writing, ``yarn`` has a bug with the ``script-shell`` config. A fix has already been committed and should make their next non-patch release. After that you can use ``yarn run build`` or just ``yarn build`` if you prefer.
At the time of this writing, ``yarn`` has a [bug](https://github.com/yarnpkg/yarn/issues/5699) with the ``script-shell`` config. A fix has already been committed and should make their next non-patch release. After that you can use ``yarn run build`` or just ``yarn build`` if you prefer.

docs/windows_development.rst Outdated Show resolved Hide resolved
@wallw-teal
Copy link
Contributor Author

  1. I'm able to build just fine with the git defaults. Do yours differ from this list?
$ git config --list
core.symlinks=false
core.autocrlf=true
core.fscache=true
color.diff=auto
color.status=auto
color.branch=auto
color.interactive=true
help.format=html
rebase.autosquash=true
http.sslcainfo=C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt
http.sslbackend=openssl
diff.astextplain.textconv=astextplain
filter.lfs.clean=git-lfs clean -- %f
filter.lfs.smudge=git-lfs smudge -- %f
filter.lfs.process=git-lfs filter-process
filter.lfs.required=true
credential.helper=manager
  1. That is not related to getting the build running in Windows and is covered elsewhere.

Copy link
Collaborator

@justin-bits justin-bits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Did you do a fresh clone of the repositories? The line endings are wrong without that setting. I started with a fresh install of git too, so I had the default config when I originally encountered the errors during build.
    core.autocrlf=true, clone opensphere-yarn-workspace, clone opensphere, yarn install, npm run lint:js
    image

core.autocrlf=input, clone opensphere-yarn-workspace, clone opensphere, yarn install, npm run lint:js
image

(no errors)

  1. Agree.

@wallw-teal
Copy link
Contributor Author

wallw-teal commented Aug 12, 2019

Yes, it was a fresh clone. I did do the original clone in cygwin though.

Edit: That's the difference. Cygwin, being a full POSIX environment, uses linux-style line endings. I'll add a .gitattributes file forcing LF for text files and a note about the core.autocrlf option in the docs.

@wallw-teal wallw-teal merged commit 8ed2c5e into ngageoint:master Aug 13, 2019
@wallw-teal wallw-teal deleted the docs-update-windows-instructions branch August 13, 2019 14:47
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.

2 participants