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

Simplify multiple path support in go plugin #1284

Merged
merged 36 commits into from
Nov 20, 2019
Merged

Simplify multiple path support in go plugin #1284

merged 36 commits into from
Nov 20, 2019

Conversation

cornfeedhobo
Copy link
Member

This PR seeks to simplify the wrapped pathmunge logic setup in #1267.
Tests have been updated to account for cases with spaces in $PATH.

@cornfeedhobo
Copy link
Member Author

Not sure why the tests failed yet; they pass locally.

@cornfeedhobo
Copy link
Member Author

The output is swallowed when run on travis; not sure how to debug.
Running the tests locally on a linux laptop passes fine

test/run test/plugins/go.plugin.bats
 ✓ plugins go: single entry in GOPATH
 ✓ plugins go: single entry in GOPATH, with space
 ✓ plugins go: single entry in GOPATH, with escaped space
 ✓ plugins go: multiple entries in GOPATH
 ✓ plugins go: multiple entries in GOPATH, with space
 ✓ plugins go: multiple entries in GOPATH, with escaped space

6 tests, 0 failures

Any suggestions?

@cornfeedhobo
Copy link
Member Author

@nwinkler As you can see, I've tried a bunch of stupid stuff to explore travis and try to make it happy, but I'm completely stumped at this point. the tests pass locally, and they pass on osx. Would you please take a look and tell me what you think?

@nwinkler
Copy link
Member

nwinkler commented Jul 8, 2019

Hi, sorry that you have to go through that amount of pain... I was on vacation for the last two weeks.

I'll take a look at this in the next couple of days, trying to replicate the issue. I'm running macOS as well, will try a VM for the Linux stuff...

@cornfeedhobo
Copy link
Member Author

cornfeedhobo commented Jul 13, 2019

What's crazy is that the CI says macOS works, and I can say from my testing and experience it works on linux (what I dev on most days). I looked at the ruby plugin tests, but I don't see a meaningful difference in how we are verifying PATH.

@nwinkler
Copy link
Member

Sorry, haven't had time to look into this yet - life got in the way...

@cornfeedhobo
Copy link
Member Author

No sweat. That's pretty much my entire last year.

@cornfeedhobo
Copy link
Member Author

cornfeedhobo commented Nov 7, 2019

@nwinkler TRAVIS FINALLY FIXED THE ISSUE AND THE BUILD PASSES. YAAAY!!!
Please, for the love of cthulhu, squash merge this disaster of a commit history.

@nwinkler nwinkler merged commit 377f027 into Bash-it:master Nov 20, 2019
@nwinkler
Copy link
Member

Woohoo!!!!! Thanks for the patience!

(Sorry about the delay, work and life got in the way the last couple of weeks...)

@cornfeedhobo
Copy link
Member Author

Likewise! Thanks for your patience as well.

@cornfeedhobo cornfeedhobo deleted the fix-go-pathmunge branch November 20, 2019 18:05
kigster pushed a commit to kigster/bash-it that referenced this pull request May 9, 2020
* simplify wrapped pathmunge logic. update tests to account for cases with spaces in $PATH.
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