support for Windows builds in make install step #80
Conversation
* return 204 on nil error response * allow void view actions * fix controller test for 204 nil error response * Fix `TestEmptyActionWithView`
syke99
changed the title
Makefile
Outdated
| ifeq ($(OS), Windows_NT) | ||
| cd livebud | ||
| npm install | ||
| cd .. | ||
| else | ||
| (cd livebud && npm install) | ||
| endif |
There was a problem hiding this comment.
| ifeq ($(OS), Windows_NT) | |
| cd livebud | |
| npm install | |
| cd .. | |
| else | |
| (cd livebud && npm install) | |
| endif | |
| @ # Note: We don't use a subshell here to support Windows. See #79. | |
| cd livebud | |
| npm install | |
| cd .. |
I'd like to avoid conditionally checking the OS if we can. Since the windows version also works on linux and mac, let's just use that. I also just added a note to my future self not to accidentally change it back 😅
There was a problem hiding this comment.
Definitely understandable!! I just pushed up that change 😋
There was a problem hiding this comment.
Thanks @syke99! By the way, next time you can just hit the "Commit suggestion" button if the change works for you 😌
There was a problem hiding this comment.
Ahh, didn't even see that option until now 😂 Thanks, I'll use it next time 😋
* return 204 on nil error response * allow void view actions * fix controller test for 204 nil error response * Fix `TestEmptyActionWithView`
|
For some reason this PR contains a6a8b12 Could you maybe rebase and push back up? |
Makefile
Outdated
| @@ -9,7 +9,10 @@ precommit: test.dev | |||
| install: | |||
| go mod tidy | |||
| npm install | |||
| (cd livebud && npm install) | |||
| # Note: We don't use a subshell here to support Windows. See #79. | |||
There was a problem hiding this comment.
This should be tabbed and be prefixed with @ so it doesn't show up when you run make install
|
Okay, repo has been rebased, comment corrected in Makefile, and changes pushed up~~ 😋 |
|
Hey @syke99, still seeing a bunch of files being updated. Maybe it's easier to close this PR, pull latest and open a new one? |
|
Sounds good, not sure why those commits are tagged to this PR. But I'll get a new one out!! |
This PR adds support for the install step on Windows for preparation of supporting Windows development.