Status icons for Pods visible on list and resources pages #784

digitalfishpond · 2016-05-31T08:12:15Z

cards for pods display an icon related to their podPhase property ('Failed', 'Pending' or 'Running') in lists and resources pages

This change is

codecov-io · 2016-05-31T08:19:14Z

Current coverage is 94.94%

Merging #784 into master will increase coverage by 0.41%

File ...ereader_directive.js (not in diff) was modified. more
- Misses -6
- Partials 0
- Hits +6

@@             master       #784   diff @@
==========================================
  Files           187        187          
  Lines          1460       1462     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1380       1388     +8   
+ Misses           80         74     -6   
  Partials          0          0

Powered by Codecov. Last updated by b5263fc...919e46b

maciaszczykm · 2016-05-31T09:47:44Z

Review status: 0 of 25 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

build/i18n.js, line 40 [r2] (raw file):

  let command = `java -jar ${conf.paths.xtbgenerator} --lang ${langKey}` +
      ` --xtb_output_file ${translationBundle}` +
      ` --js ${codeSource}`;

You can move it into one line:
--xtb_output_file ${translationBundle} --js ${codeSource};

src/app/frontend/common/components/endpoint/endpoint_module.js, line 17 [r2] (raw file):

import {externalEndpointComponent} from './externalendpoint_component';
import {internalEndpointComponent} from './internalendpoint_component';

Too many empty lines in a row. Please check other files too.

src/test/frontend/deploy/createsecret_controller_test.js, line 73 [r2] (raw file):

            `vdjEvIjogeyAiYXV0aCI6ICJabUZyWlhCaG` +
            `MzTjNiM0prTVRJSyIsICJlbWFpbCI6` +
            `ICJqZG9lQGV4YW1wbGUuY29tIiB9IH0K`)

You can squeeze it into two lines:

expect((`eyAiaHR0cHM6Ly9pbmRleC5kb2NrZXIuaW8vdjEvIjogeyAiYXV0aCI6ICJabUZyWlhCaG` +
`MzTjNiM0prTVRJSyIsICJlbWFpbCI6ICJqZG9lQGV4YW1wbGUuY29tIiB9IH0K`)

Comments from Reviewable

bryk · 2016-05-31T10:02:09Z

Reviewed 4 of 7 files at r1, 9 of 20 files at r2.
Review status: 13 of 25 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

src/app/frontend/podlist/podcardlist.html, line 37 [r2] (raw file):

      <md-icon class="material-icons kd-error" ng-if="::pod.podPhase=='Failed'">
        error
        <md-tooltip md-direction="right">One or more pods have errors</md-tooltip>

This is exacly one pod. Rewrite the message please :)

src/app/frontend/podlist/podcardlist.html, line 41 [r2] (raw file):

      <md-icon class="material-icons" ng-if="::pod.podPhase=='Pending'">
        timelapse
        <md-tooltip md-direction="right">One or more pods are in pending state</md-tooltip>

This is exacly one pod. Rewrite the message please :)

src/app/frontend/podlist/podcardlist.html, line 41 [r2] (raw file):

      <md-icon class="material-icons" ng-if="::pod.podPhase=='Pending'">
        timelapse
        <md-tooltip md-direction="right">One or more pods are in pending state</md-tooltip>

Btw, can you use MSG_FOO internationalization framework. Consult @taimir if dont know how.

src/app/frontend/podlist/podcardlist_component.js, line 66 [r2] (raw file):

   */
  hasWarnings() {
    console.log(this);

Remove?

src/app/frontend/podlist/podcardlist_component.js, line 67 [r2] (raw file):

  hasWarnings() {
    console.log(this);
    return pod.running;

return pod.running === 0? This would be easier to understand :) At first look I thought this was a mistake.

Comments from Reviewable

digitalfishpond · 2016-05-31T13:42:23Z

Review status: 7 of 24 files reviewed at latest revision, 7 unresolved discussions.

src/app/frontend/podlist/podcardlist.html, line 41 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Btw, can you use MSG_FOO internationalization framework. Consult @taimir if dont know how.

@taimir, could I ask you to have a look and tell me what I'm doing wrong here please? This string is set in .../podlist/podcardlist_component.js

Comments from Reviewable

taimir · 2016-06-01T07:29:20Z

Review status: 4 of 9 files reviewed at latest revision, 9 unresolved discussions.

src/app/frontend/podlist/podcardlist.html, line 41 [r2] (raw file):

Previously, digitalfishpond (Denis Poisson) wrote…

@taimir, could I ask you to have a look and tell me what I'm doing wrong here please? This string is set in .../podlist/podcardlist_component.js

AFAICS, what's not right is that you don't display any text in your tooltips?

You are doing nothing wrong with the definition of the MSG variables. The problem is that your controller is not accessible in the tooltip (not sure why, you'll have to investigate this). E.g. {{ctrl == undefined}} displays true at the place where you want to access the MSG_ variables in the html code.

src/app/frontend/podlist/podcardlist_component.js, line 87 [r4] (raw file):

const i18n = {
  controller: PodCardListController,

Do you need this here? Normally the const object only contains the message variables.

Comments from Reviewable

digitalfishpond · 2016-06-01T07:39:20Z

Thanks to @taimir for the pointers :) PTAL

Previously, codecov-io wrote…

[Current coverage][cc-pull] is 94.92%

Merging [#784][cc-pull] into [master][cc-base-branch] will increase coverage by 0.41%

File ...ereader_directive.js (not in diff) was modified. more

Misses -6

Partials 0

Hits +6
@@             master       #784   diff @@
==========================================
  Files           187        187          
  Lines          1454       1456     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1374       1382     +8   
+ Misses           80         74     -6   
  Partials          0          0          
Powered by Codecov. Last updated by [45f56f9...d93a3f7][cc-compare]
[cc-base-branch]: https://codecov.io/gh/kubernetes/dashboard/branch/master?src=pr
[cc-compare]: https://codecov.io/gh/kubernetes/dashboard/compare/45f56f976c482a576612db168f8dc80b5bfef0c8...d93a3f723b428eefed66e5172750636842c98e83
[cc-pull]: https://codecov.io/gh/kubernetes/dashboard/pull/784?src=pr

Comments from Reviewable

digitalfishpond · 2016-06-01T07:39:38Z

Review status: 4 of 9 files reviewed at latest revision, 9 unresolved discussions.

src/app/frontend/podlist/podcardlist.html, line 41 [r2] (raw file):

Previously, taimir (Atanas Mirchev) wrote…

AFAICS, what's not right is that you don't display any text in your tooltips?

You are doing nothing wrong with the definition of the MSG variables. The problem is that your controller is not accessible in the tooltip (not sure why, you'll have to investigate this). E.g. {{ctrl == undefined}} displays true at the place where you want to access the MSG_ variables in the html code.

Ah :) Thanks for the clue!

Comments from Reviewable

bryk · 2016-06-01T09:40:21Z

Reviewed 6 of 7 files at r4, 1 of 2 files at r5.
Review status: 8 of 9 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

Comments from Reviewable

bryk · 2016-06-01T09:55:26Z

We have conflicts. Please rebase :)

Previously, digitalfishpond (Denis Poisson) wrote…

Thanks to @taimir for the pointers :) PTAL

Review status: 8 of 9 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

Comments from Reviewable

digitalfishpond · 2016-06-01T10:34:28Z

PTAL

Previously, bryk (Piotr Bryk) wrote…

We have conflicts. Please rebase :) :lgtm:

Review status: 6 of 9 files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable

bryk · 2016-06-01T11:08:12Z

Previously, digitalfishpond (Denis Poisson) wrote…

PTAL

Reviewed 1 of 2 files at r5, 2 of 2 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

googlebot added the cla: yes label May 31, 2016

digitalfishpond force-pushed the pods-status-icons branch from 0ef2423 to 8864496 Compare May 31, 2016 08:23

digitalfishpond force-pushed the pods-status-icons branch 8 times, most recently from d0b0fad to b198bd0 Compare May 31, 2016 13:38

digitalfishpond force-pushed the pods-status-icons branch 2 times, most recently from 6096310 to aa32bac Compare June 1, 2016 07:18

digitalfishpond force-pushed the pods-status-icons branch from aa32bac to 71a4260 Compare June 1, 2016 07:36

Status icons for Pods visible on list and resources pages

3cfd3a4

digitalfishpond force-pushed the pods-status-icons branch from 71a4260 to 3cfd3a4 Compare June 1, 2016 10:33

bryk merged commit 25dac01 into kubernetes:master Jun 1, 2016

digitalfishpond deleted the pods-status-icons branch June 10, 2016 07:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Status icons for Pods visible on list and resources pages #784

Status icons for Pods visible on list and resources pages #784

Uh oh!

digitalfishpond commented May 31, 2016 •

edited by bryk

Loading

Uh oh!

codecov-io commented May 31, 2016 •

edited

Loading

Uh oh!

maciaszczykm commented May 31, 2016

Uh oh!

bryk commented May 31, 2016

Uh oh!

digitalfishpond commented May 31, 2016

Uh oh!

taimir commented Jun 1, 2016

Uh oh!

digitalfishpond commented Jun 1, 2016

[Current coverage][cc-pull] is 94.92%

Uh oh!

digitalfishpond commented Jun 1, 2016

Uh oh!

bryk commented Jun 1, 2016

Uh oh!

bryk commented Jun 1, 2016

Uh oh!

digitalfishpond commented Jun 1, 2016

Uh oh!

bryk commented Jun 1, 2016

Uh oh!

Uh oh!

Status icons for Pods visible on list and resources pages #784

Status icons for Pods visible on list and resources pages #784

Uh oh!

Conversation

digitalfishpond commented May 31, 2016 • edited by bryk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented May 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 94.94%

Uh oh!

maciaszczykm commented May 31, 2016

Uh oh!

bryk commented May 31, 2016

Uh oh!

digitalfishpond commented May 31, 2016

Uh oh!

taimir commented Jun 1, 2016

Uh oh!

digitalfishpond commented Jun 1, 2016

[Current coverage][cc-pull] is 94.92%

Uh oh!

digitalfishpond commented Jun 1, 2016

Uh oh!

bryk commented Jun 1, 2016

Uh oh!

bryk commented Jun 1, 2016

Uh oh!

digitalfishpond commented Jun 1, 2016

Uh oh!

bryk commented Jun 1, 2016

Uh oh!

Uh oh!

digitalfishpond commented May 31, 2016 •

edited by bryk

Loading

codecov-io commented May 31, 2016 •

edited

Loading