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

[YUNIKORN-1280] Shim side changes for YUNIKORN-1275 #450

Closed
wants to merge 4 commits into from

Conversation

manirajv06
Copy link
Contributor

What is this PR for?

Shim side changes to support arbitrary resources in namespace annotation.

What type of PR is it?

  • - Improvement

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-1280

How should this be tested?

Screenshots (if appropriate)

Questions:

  • - The licenses files need update.
  • - There is breaking changes for older versions.
  • - It needs documentation.

@manirajv06 manirajv06 self-assigned this Aug 18, 2022
Copy link
Contributor

@wilfred-s wilfred-s left a comment

Choose a reason for hiding this comment

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

Can we split the tests for annotations into 3 tests:

  • test annotations based on the new setup (new function)
  • mixed new and deprecated (new function)
  • deprecated annotation (existing test)

Makes checks and regressions easier to track down.

pkg/common/utils/utils.go Outdated Show resolved Hide resolved
test/e2e/queue_quota_mgmt/queue_quota_mgmt_test.go Outdated Show resolved Hide resolved
@manirajv06
Copy link
Contributor Author

Can we split the tests for annotations into 3 tests:

  • test annotations based on the new setup (new function)
  • mixed new and deprecated (new function)
  • deprecated annotation (existing test)

Makes checks and regressions easier to track down.

Taken care

@manirajv06 manirajv06 closed this Aug 22, 2022
@manirajv06 manirajv06 reopened this Aug 22, 2022
@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Merging #450 (3ebbcbd) into master (fdf455b) will decrease coverage by 0.24%.
The diff coverage is 38.00%.

@@            Coverage Diff             @@
##           master     #450      +/-   ##
==========================================
- Coverage   67.24%   67.00%   -0.25%     
==========================================
  Files          41       41              
  Lines        6722     6767      +45     
==========================================
+ Hits         4520     4534      +14     
- Misses       2032     2062      +30     
- Partials      170      171       +1     
Impacted Files Coverage Δ
pkg/common/resource.go 81.42% <0.00%> (-12.89%) ⬇️
pkg/common/utils/utils.go 58.89% <76.00%> (+1.55%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@wilfred-s wilfred-s left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants