-
Notifications
You must be signed in to change notification settings - Fork 986
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
Update dep with Knative dependency and add knative service placeholder #10
Conversation
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 assume you just needed to reference knative to ensure the deps were all healthy.
/lgtm
/approval
@ellis-bigelow sorry can you lgtm again ? I removed an unused dependency. |
|
||
[[constraint]] | ||
name = "github.com/knative/serving" | ||
version = "v0.5.0" |
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.
Why are we locking this version w/ a constraint? In my prototype I put this as a requirement.
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 thought it is good to lock the version, since required
may find the latest version and your current code may break, maybe I do not fully understand how required
works.
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: Consider "fixup" on your commits to keep the history more clean.
cleaned up. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ellis-bigelow The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I read through https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md again and I think you're doing it right :) |
kserve#10) * update dep dependency for knative * watch knative service and generate rbac
inferenceservice api reference doc kserve#10
Both workflows for PRs and pushes were cleaned up. On a push (which includes when PRS merge), the code base is linted and tested first before building and publishing.
Downgrade py-spy to "0.3.12"
Add knative dependency to Gopkg.toml, knative is on
kube 1.12
and the latestcontroller-runtime
version forkube 1.12
is0.1.9
Add knative service for placeholder in main controller reconcile function
Add vendor files and the only files interested to review are
Gopkg.toml
andkfservice_controller.go
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)