Skip to content
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

Resource limits frontend implementation #1292

Merged

Conversation

batikanu
Copy link
Contributor

@batikanu batikanu commented Sep 28, 2016

Frontend implementation for resource limits part of namespace details.
Includes also backend fixes

@batikanu batikanu force-pushed the namespace-detail-resource-limits-frontend branch from 58ab4ea to 720bda0 Compare September 28, 2016 14:21
@codecov-io
Copy link

codecov-io commented Sep 28, 2016

Current coverage is 93.49% (diff: 100%)

Merging #1292 into master will increase coverage by 0.01%

@@             master      #1292   diff @@
==========================================
  Files           366        368     +2   
  Lines          3037       3046     +9   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2839       2848     +9   
  Misses          198        198          
  Partials          0          0          

Powered by Codecov. Last update 3a492dd...d65edef

@floreks
Copy link
Member

floreks commented Sep 30, 2016

Please add related section to admin page.

@batikanu
Copy link
Contributor Author

Please see #1285 for admin section. PTAL

@batikanu batikanu force-pushed the namespace-detail-resource-limits-frontend branch from 720bda0 to d473565 Compare October 12, 2016 12:41
@batikanu batikanu changed the title Namespace detail resource limits frontend resource limits frontend implementation Oct 12, 2016
@batikanu batikanu changed the title resource limits frontend implementation Resource limits frontend implementation Oct 12, 2016
@batikanu batikanu force-pushed the namespace-detail-resource-limits-frontend branch 3 times, most recently from d8d8f23 to 694143f Compare October 21, 2016 11:29
@batikanu batikanu force-pushed the namespace-detail-resource-limits-frontend branch 2 times, most recently from 14ec586 to 8756442 Compare November 8, 2016 08:32
@bryk
Copy link
Contributor

bryk commented Nov 21, 2016

@batikanu Is this ready for review? Somehow it got stuck in our review queue and nobody looks at this. Can you rebase so that me or somebody else can review? :)

Sorry for such delay, btw...

@batikanu
Copy link
Contributor Author

Yes sure I will rebase it :)

@bryk bryk self-assigned this Nov 21, 2016
@batikanu batikanu force-pushed the namespace-detail-resource-limits-frontend branch from 8756442 to b50d593 Compare November 21, 2016 13:41
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 21, 2016
@batikanu
Copy link
Contributor Author

@bryk this should be ready for review if nothing fails in travis :)

@bryk
Copy link
Contributor

bryk commented Nov 21, 2016

:)

Copy link
Contributor

@bryk bryk left a comment

Choose a reason for hiding this comment

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

A few cosmetic comments over look and feel. Will review code throughly tomorrow. Otherwise great work! :)

Can you fix the double header? Look at other pages which did this

image

Can you add dashes to empty value cells?

image

@batikanu batikanu force-pushed the namespace-detail-resource-limits-frontend branch from b50d593 to d65edef Compare November 21, 2016 15:44
@batikanu
Copy link
Contributor Author

Double header is solved and dashes are added :)

@bryk
Copy link
Contributor

bryk commented Nov 22, 2016

LGTM! I'm sad this took so long to review...

@bryk bryk merged commit 13fe142 into kubernetes:master Nov 22, 2016
@bryk bryk deleted the namespace-detail-resource-limits-frontend branch November 22, 2016 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants