-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Feature scope #1949
Feature scope #1949
Conversation
Address string `json:"address"` | ||
} | ||
|
||
// GetScope returns a populated Scope struct. |
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.
Copy-pasted comment?
src/app/frontend/scope/controller.js
Outdated
* @export | ||
*/ | ||
pingScope(tries) { | ||
if (tries < 5) { |
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.
Calling pingScope(5) will do nothing, right? I initially was a little confused by this.
Can you refactor this argument (e.g. to decrease until 0), or add a comment about its meaning?
Codecov Report
@@ Coverage Diff @@
## master #1949 +/- ##
==========================================
- Coverage 59.08% 58.71% -0.38%
==========================================
Files 582 587 +5
Lines 12171 12328 +157
==========================================
+ Hits 7191 7238 +47
- Misses 4811 4919 +108
- Partials 169 171 +2
Continue to review full report at Codecov.
|
15d1bfb
to
89d40e2
Compare
func PostScope(client client.Interface) (*Scope, error) { | ||
log.Print("Installing scope on the cluster") | ||
|
||
response, err := http.Get("https://cloud.weave.works/launch/k8s/weavescope.yaml?k8s-service-type=NodePort") |
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 is the part I'm worried about. It allows cloud.weave.works to hack user's cluster. Can we embed the YAML in repository here and versionize it? It'd be a bit safer at least.
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.
Issues go stale after 90d of inactivity. Prevent issues from auto-closing with an If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or |
Let's get back to this! /remove-lifecycle stale |
@errordeveloper I think it is not the best time. We are migrating the whole app to new Angular and it would be better to wait for it. Let's wait for #2727 and return to this discussion then. |
Thanks, Marcin! We shall wait.
…On Sat, 20 Jan 2018, 3:51 pm Marcin Maciaszczyk, ***@***.***> wrote:
@errordeveloper <https://github.com/errordeveloper> I think it is not the
best time. We are migrating the whole app to new Angular and there is no
sense in implementing it for AngularJS. Let's wait for #2727
<#2727> and return to this
discussion then.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1949 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPWS5gfs2FqmHselsm1-UtkZWn5jACrks5tMguZgaJpZM4Na9E4>
.
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
As per #1577