Skip to content
This repository has been archived by the owner on Mar 14, 2022. It is now read-only.

Repair swagger #76

Closed
wants to merge 5 commits into from
Closed

Repair swagger #76

wants to merge 5 commits into from

Conversation

cmharlow
Copy link
Contributor

Regenerating API code to work. Removed status from Resource model, since status isn't yet determined as something the API code needs to be aware of.

Dependencies (UUID library) are missing from this PR (are getting put into $GOPATH via go get and not into dep ensure) that need to be documented and added. We also have a dep / aws-sdk for go conflict. Opened issues #75 #74 for these, respectively.

@cmharlow
Copy link
Contributor Author

Also moves up in priority #42 and #73 (not just dep version, but go-swagger version)

"example" : "0a72c00a-1332-4730-b19c-9131c3bcb57f"
}
}
"$ref" : "#/definitions/Resource"
Copy link
Contributor

@jcoyne jcoyne Jan 26, 2018

Choose a reason for hiding this comment

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

I don't agree that this is the correct response for a create. As documented on the whiteboard the response should be a RequestID, a ResourceID and a RequestStatus. I don't think it makes sense to send them back the resource they just sent us. See #44

@jcoyne
Copy link
Contributor

jcoyne commented Jan 26, 2018

I'm curious to know what you mean about the UUID library being missing. I see it in the lockfile right here: https://github.com/sul-dlss-labs/taco/blob/master/Gopkg.lock#L156-L159

@cmharlow
Copy link
Contributor Author

cmharlow commented Jan 26, 2018 via email

@jcoyne
Copy link
Contributor

jcoyne commented Jan 26, 2018

@cmh2166 are we certain it needs to be in the toml file? I thought that was just for the ones it can't introspect. The true list of dependencies is in the .lock file.

@cmharlow
Copy link
Contributor Author

Check the docs around dep ensure -update: https://golang.github.io/dep/docs/daily-dep.html

@jcoyne
Copy link
Contributor

jcoyne commented Jan 26, 2018

@cmh2166 yeah on that page I'm seeing:

Of course, given this model, you don't have to use dep ensure -add to add new dependencies - you can also just add an appropriate import statement in your code, then run dep ensure. However, this approach doesn't always play nicely with goimports, and also won't append a [[constraint]] into Gopkg.toml. Still, it can be useful at times, often for rapid iteration and off-the-cuff experimenting.

So I'm wondering what the drawback to this approach is. This also seems to be what one of the maintainers recommends as her preferred workflow: golang/dep#376 (comment)

@cmharlow
Copy link
Contributor Author

@jcoyne i'm not commenting on the approach against, i'm saying we don't do that cleanly right now (hence your original PR adding all this dynamo stuff not working when I pulled down and only ran dep ensure).

Regardless, that is ticketed and does not stop this PR.

@cmharlow
Copy link
Contributor Author

Closing ticket given recent updates. Doesn't close issues related since those are still undetermined (and the code / spec still need cleaning up).

@cmharlow cmharlow closed this Jan 29, 2018
@cmharlow cmharlow removed the review label Jan 29, 2018
@cmharlow cmharlow deleted the repair_swagger branch January 29, 2018 23:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants