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

Add generic client interfaces to core/resource API #67

Merged
merged 8 commits into from
Nov 17, 2021

Conversation

kstiehl
Copy link
Contributor

@kstiehl kstiehl commented Nov 4, 2021

Description

Make resource compliant with generic API interfaces{}

Release Note

Release note for CHANGELOG:

* core/resources - make resource compliant with generic API interfaces

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@kstiehl kstiehl force-pushed the feature/resource-new-generic-api branch from 0448934 to 5876ac7 Compare November 4, 2021 13:11
@kstiehl kstiehl marked this pull request as ready for review November 4, 2021 15:21
@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2021

Codecov Report

Merging #67 (3437b84) into main (53e2c92) will increase coverage by 0.00%.
The diff coverage is 64.70%.

❗ Current head 3437b84 differs from pull request most recent head 6c593b3. Consider uploading reports for the commit 6c593b3 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main      #67   +/-   ##
=======================================
  Coverage   55.06%   55.07%           
=======================================
  Files          87       87           
  Lines        3634     3648   +14     
=======================================
+ Hits         2001     2009    +8     
- Misses       1306     1310    +4     
- Partials      327      329    +2     
Flag Coverage Δ
integration 44.85% <64.70%> (+1.12%) ⬆️
unittests 27.33% <0.00%> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/core/resource/resource.go 14.28% <ø> (ø)
pkg/core/resource/api.go 68.42% <64.70%> (-31.58%) ⬇️
pkg/api/object.go 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53e2c92...6c593b3. Read the comment docs.

@LittleFox94
Copy link
Contributor

Also maybe change all the Describe, Context and It strings in the tests to start lowercase as they are expected

Actual logic lgtm

pkg/core/resource/api.go Show resolved Hide resolved
pkg/core/resource/api.go Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
tests/core_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@LittleFox94 LittleFox94 left a comment

Choose a reason for hiding this comment

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

lgtm

@LittleFox94
Copy link
Contributor

Please rebase onto master and then feel free to merge :)

@kstiehl kstiehl force-pushed the feature/resource-new-generic-api branch from 3437b84 to 6c593b3 Compare November 5, 2021 10:07
@kstiehl kstiehl force-pushed the feature/resource-new-generic-api branch 2 times, most recently from 8d31250 to d91b20e Compare November 17, 2021 18:58
@kstiehl kstiehl force-pushed the feature/resource-new-generic-api branch from d91b20e to f145e80 Compare November 17, 2021 19:09
@kstiehl kstiehl merged commit 4145a35 into main Nov 17, 2021
@kstiehl kstiehl deleted the feature/resource-new-generic-api branch November 17, 2021 19:21
@LittleFox94 LittleFox94 mentioned this pull request Mar 28, 2022
1 task
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.

None yet

3 participants