-
Notifications
You must be signed in to change notification settings - Fork 42
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
moved everything and got dep to pass #14
Conversation
Gopkg.toml
Outdated
revision = "6b3b338d5f9c8b5a80ad4ea1e2e37aa58677ea9d" | ||
name = "github.com/vektah/gqlgen" | ||
|
||
# this must match the version of the gqlgen binary we depend on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment suggests we should be pinning the gqlgen binary, which I don't see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That comment has to do with the API server, it's left over. Sqoop does not use the binary
api/v1/resolver_map.proto
Outdated
} | ||
|
||
// A Go-template which will return data for a Resolver without making a function call. Template Resolvers can make use | ||
// of Sqoop's builtin template functions as well as the data provided by the Params object to the resolver. | ||
// Read more about Templates and Resolvers in Sqoop's [Resolver documentation](TODO). | ||
// Read more about Templates and Resolvers in Sqoop\'s [Resolver documentation](TODO). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove TODO from proto because it will be published in our docs. (Remove all TODOs, if there are others). File issue to track instead.
api/v1/schema.proto
Outdated
option (core.solo.io.resource).short_name = "sc"; | ||
option (core.solo.io.resource).plural_name = "schemas"; | ||
// TODO: remove when we are ready to include this resource in the docs | ||
option (core.solo.io.resource).skip_docs_gen = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't skip docs gen
ci/check-code-and-docs-gen.sh
Outdated
exit 1; | ||
fi | ||
|
||
(cd projects/gloo/doc && make site -B) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right
mkdocs.yml
Outdated
@@ -31,7 +31,7 @@ site_author: gloo Project Authors | |||
copyright: © Copyright 2018, solo.io Inc. | |||
google_analytics: ['UA-101654320-1', 'sqoop.solo.io'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to switch to Hugo
test/testdata/utils.go
Outdated
// resolverMap.Types["Query"].Fields["droid"].Resolver = &v1.Resolver_GlooResolver{ | ||
// GlooResolver: &v1.GlooResolver{ | ||
// RequestTemplate: `{"id": {{ index .Args "id" }}}`, | ||
// Function: &v1.GlooResolver_SingleFunction{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out code
// createResolver func(typeName, fieldName string) (RawResolver, error) | ||
// ) | ||
// BeforeEach(func() { | ||
// requestBody = &bytes.Buffer{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out code
test/unit/gloo_resolvers_test.go
Outdated
// | ||
// "github.com/gorilla/mux" | ||
// "github.com/solo-io/sqoop/pkg/api/v1" | ||
// . "github.com/solo-io/sqoop/pkg/engine/resolvers/gloo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out code
test/unit/router_test.go
Outdated
package unit_test | ||
|
||
//import ( | ||
// . "github.com/onsi/ginkgo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commented out code
hack/create-release.sh github_api_token=$(GITHUB_TOKEN) owner=solo-io repo=sqoop tag=v$(VERSION) | ||
@$(foreach BINARY,$(RELEASE_BINARIES),hack/upload-github-release-asset.sh github_api_token=$(GITHUB_TOKEN) owner=solo-io repo=sqoop tag=v$(VERSION) filename=$(BINARY);) | ||
release: release-binaries release-yamls | ||
ifeq ($(RELEASE),"true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could add the docs push in this PR or in a follow up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err let's do it here, i see we have the script already
ci/check-code-and-docs-gen.sh
Outdated
exit 1; | ||
fi | ||
|
||
(cd sqoop/doc && make site -B) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be removed
@@ -7,7 +7,7 @@ import ( | |||
. "github.com/onsi/gomega" | |||
) | |||
|
|||
func TestGloo(t *testing.T) { | |||
func TestResolvermap(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should be ResolverMap here and below
cli/pkg/helpers/clients.go
Outdated
if err != nil { | ||
return nil, errors.Wrapf(err, "getting kube config") | ||
} | ||
secretClient, err := glooV1.NewSecretClient(&factory.KubeSecretClientFactory{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs KubeCoreCache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but I also don't use that code so I removed it
No description provided.