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

x/playground: add vet #7597

Closed
josharian opened this issue Mar 20, 2014 · 23 comments
Closed

x/playground: add vet #7597

josharian opened this issue Mar 20, 2014 · 23 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@josharian
Copy link
Contributor

On potential pain of button overload, it might be useful to add a Vet button to the
playground. It would run go vet over the program and display the output. This would
increase vet visibility and help make self-service debugging easier. (E.g. It would have
helped with https://groups.google.com/forum/m/#!topic/golang-nuts/IneZcyGGJYM )
@adg
Copy link
Contributor

adg commented Mar 21, 2014

Comment 1:

Not a bad idea.

Owner changed to @adg.

Status changed to Accepted.

@adg
Copy link
Contributor

adg commented Mar 31, 2014

Comment 2:

Labels changed: added repo-playground, release-none.

@josharian
Copy link
Contributor Author

Comment 3:

Is this something that I can work on? If so, do you mind pointing me in the right
direction? (Where is the relevant codebase? Anything I need to be aware of for
testing/portability to the hosted playground environment?)

@adg
Copy link
Contributor

adg commented May 30, 2014

Comment 4:

Please go ahead.
http://code.google.com/p/go-playground is where the code lives. Take a look at how the
fmt handler works. It would be similar to that.

@josharian
Copy link
Contributor Author

Comment 5:

Thanks. The front-end part was surprisingly pleasant and straightforward.
Sadly, the rest is going to be disruptive. It will require converting vet into a
package. I have a couple of vet modules queued for 1.4. Once those land (or crash), I'll
email golang-dev to see how folks feel about turning vet into a package.

Owner changed to @josharian.

Status changed to Started.

@cookieo9
Copy link
Contributor

cookieo9 commented Jul 9, 2014

Comment 6:

I like this as it would hopefully reduce some of the traffic to golang-nuts.
It would also be nice if there was a race option for the run button, like the imports
option on the Format button. It would hopefully have a similar effect to bring more
visibility to the race-detector.
That may not be possible though, since the race detector probably won't work on the
playground platform. AFAIK it only works on 64-bit darwin/linux/windows/freebsd, while
the playground is nacl/386 I believe.

@minux
Copy link
Member

minux commented Jul 9, 2014

Comment 7:

FYI, the playground is nacl/amd64p32.
Due to the way the race detector works and nacl works, it's impossible
to support the race detector on nacl.

@josharian
Copy link
Contributor Author

The front-end work I did for this (now perhaps a bit dated) can be found at https://codereview.appspot.com/103750044/ and https://codereview.appspot.com/104770043/. The hard part--splitting vet into a package and a command--is still outstanding.

@adg
Copy link
Contributor

adg commented Jan 11, 2015

Now that the new sandbox server is running on a VM, we can think about just
calling the vet command directly.

On 10 January 2015 at 06:47, Josh Bleecher Snyder notifications@github.com
wrote:

The front-end work I did for this (now perhaps a bit dated) can be found
at https://codereview.appspot.com/103750044/ and
https://codereview.appspot.com/104770043/. The hard part--splitting vet
into a package and a command--is still outstanding.


Reply to this email directly or view it on GitHub
#7597 (comment).

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed the release-none label Apr 10, 2015
@rsc rsc changed the title playground: add vet x/playground: add vet Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@ucirello
Copy link
Contributor

Running playground on a sandbox makes easier to execute cmd/vet on a piece of code.

These two CLs addresses this issue, by updating both sandbox, playground frontend application and a static file shared with godoc.

Currently, it is implemented following this way:
1 - Added one entry point (/vet) to sandbox, which calls cmd/vet and parses the result as if it were a compilation process.
2 - Updated playground.js, so it calls /vet at backend.
3 - Updated Dockerfile with go get to install cmd/vet

I considered the alternative of overloading /compile endpoint with a parameter, thus switching back and forth between compilation and vetting. The reason I did not do it was that it seemed a cleaner design to keep separated different resources with different goals.

Of course, I am ready to change the implementation after debate.

Anyway, I should like to open as soon as possible another CL updating docs to mention the existence of a proxy at golang.org/compile - so other contributos do not get lost on why query-string calls are made where the sandbox expects for JSON.

https://go-review.googlesource.com/#/c/11632/
https://go-review.googlesource.com/#/c/11633/

@ucirello
Copy link
Contributor

@adg ping

@ucirello
Copy link
Contributor

@josharian ping.

@gopherbot
Copy link

CL https://golang.org/cl/11633 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/11632 mentions this issue.

@adg
Copy link
Contributor

adg commented Jul 1, 2015

Sorry, I'm busy preparing for GopherCon. I will review this after next week.

On 1 July 2015 at 08:01, GopherBot notifications@github.com wrote:

CL https://golang.org/cl/11633 mentions this issue.


Reply to this email directly or view it on GitHub
#7597 (comment).

@ucirello
Copy link
Contributor

@adg Sorry to bother you, could you please take a look at these CLs?

https://golang.org/cl/11633
https://golang.org/cl/11632

@adg
Copy link
Contributor

adg commented Jul 28, 2015

After Go 1.5 is out, yes.

On 27 July 2015 at 16:23, Carlos Cirello notifications@github.com wrote:

@adg https://github.com/adg Sorry to bother you, could you please take
a look at these CLs?

https://golang.org/cl/11633
https://golang.org/cl/11632


Reply to this email directly or view it on GitHub
#7597 (comment).

@ysmolski
Copy link
Member

I am working on it. Please give me early feedback if you have it.

screen shot 2018-03-14 at 11 21 42

screen shot 2018-03-14 at 11 23 54

/CC @andybons @bradfitz

@andybons
Copy link
Member

UI lgtm!

@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 14, 2018
@gopherbot
Copy link

Change https://golang.org/cl/100775 mentions this issue: godoc: add vet support for playground

@gopherbot
Copy link

Change https://golang.org/cl/100776 mentions this issue: playground: add vet

gopherbot pushed a commit to golang/tools that referenced this issue Mar 23, 2018
Playground needs godoc to support calls to /vet endpoint in playground.js.
Optional parameter "vetEl" is added to the function "playground".
If it's passed then the js installs the click handler to the element.

There is a corresponding CL 100776 for the playground code.

Updates golang/go#7597

Change-Id: Ica2e7cb9d76f6f19a1805c182e666b8142762da9
Reviewed-on: https://go-review.googlesource.com/100775
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@ysmolski
Copy link
Member

ysmolski commented Apr 3, 2018

Reviewers are welcome here: https://golang.org/cl/100776

@gopherbot
Copy link

Change https://golang.org/cl/107455 mentions this issue: godoc: display "go vet" errors before the output of a program

gopherbot pushed a commit to golang/tools that referenced this issue May 8, 2018
This change adds an option to run "go vet" for the playground program
and display errors before any output. To enable this, the playground
function has to be supplied with opts.enableVet set to true.
Vet check is performed only for succesfully run programs,
meaning that the "/compile" endpoint returned no errors.

This change highlights lines printed to stderr as errors (in red).

There is a corresponding change for the Playground: CL 100776.

Updates golang/go#7597
Updates golang/go#24576

Change-Id: I8c0f8c1189c461338b5bce57777b12aecab268fb
Reviewed-on: https://go-review.googlesource.com/107455
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@golang golang locked and limited conversation to collaborators May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

10 participants