feat: added testFlags option - #1877
Conversation
|
|
|
Resolves: #1842 |
| } | ||
|
|
||
| let buildTags: string = testconfig.goConfig['buildTags']; | ||
| let buildTags: string = testconfig.goConfig['testTags']; |
There was a problem hiding this comment.
I believe we should fallback to using buildTags if no testTags are provided to ensure we are not breaking anybody else's use case.
There was a problem hiding this comment.
Of course!
| "type": "string", | ||
| "default": "", | ||
| "description": "The Go build tags to use for all commands that support a `-tags '...'` argument", | ||
| "description": "The Go build tags to use for all commands, excluding `go test` (see testTags), that support a `-tags '...'` argument", |
There was a problem hiding this comment.
buildtags will still be used for compiling the test files which will be using go test
So we should probably change this to excluding when tests are run or something to that effect
| } | ||
|
|
||
| let buildTags: string = testconfig.goConfig['buildTags']; | ||
| let testTags: string = (testconfig.goConfig.has("testTags")) ? testconfig.goConfig['testTags'] : testconfig.goConfig['buildTags']; |
There was a problem hiding this comment.
We need to use single quotes, else the linter breaks the build.
There was a problem hiding this comment.
oops!
| "type": "string", | ||
| "default": "", | ||
| "description": "The Go build tags to use for all commands that support a `-tags '...'` argument", | ||
| "description": "The Go build tags to use for all commands, excluding when running tests (see testTags), that support a `-tags '...'` argument", |
There was a problem hiding this comment.
The description now is a little confusing. Can we move the exclude statement to the end? How about the below?
The Go build tags to use for all commands that support a -tags '...'argument. When running tests,go.testTags will be used instead if it was set.
There was a problem hiding this comment.
That does sound better :)
|
The test with the multiple flags seems to be failing as well, not sure why since it's implemented just like |
|
The That's the reason for the failures |
|
Ah, thanks! Will get it sorted out today. Haven't had a second look yet, sorry for the delay :) |
|
No problem, take your time. |
7e2b904 to
c29017c
Compare
|
I removed the test for fallback on First I chose to modify the check to But that would create a weird dependency on Not sure how to solve the backwards compatibility problem actually since an unset config evaluated to an empty string and not undefined. |
|
Ehm, getting weird errors The function definition is as follows: Any ideas? |
Merge from the |
We had a similar case with the
|
|
That should do it! |
|
When can I expect this to get released? :) |
|
Johan Lejdung (@johan-lejdung) A release should be some time next week. Until then, you can use the beta version of the extension to use this feature. See https://github.com/Microsoft/vscode-go/wiki/Use-the-beta-version-of-the-latest-Go-extension Thanks for working on this! |
|
Super, thank you for the help!
…On Tue, 4 Sep 2018 at 08:20, Ramya Rao ***@***.***> wrote:
@johan-lejdung <https://github.com/johan-lejdung> A release should be
some time next week. Until then, you can use the beta version of the
extension to use this feature. See
https://github.com/Microsoft/vscode-go/wiki/Use-the-beta-version-of-the-latest-Go-extension
Thanks for working on this!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1877 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AI1mxJqzvLld1JWVDicqrKQgsXgBkPmsks5uXhvCgaJpZM4WM1JW>
.
|
|
Bless you for building this! I've been dying for this feature! |
|
That's really nice to hear |
4 participants
I tested it out locally and it worked fine and all tests except rungodoc passed but it looked like I had to do some setup for that to pass to that is most likely the reason.