Skip to content

Conversation

@JaviCerveraIngram
Copy link
Collaborator

Rename BaseResource's build_filter and list as get_filters and get_list respectively (the latter being now a method instead of a property). get_list now accepts optional filters (get_filters is used if none provided).

…ist respectively. get_list now accepts optional filters (get_filters is used if none provided).
@marcserrat marcserrat self-requested a review April 23, 2019 12:41
filters[key] = val
return filters

def get_list(self, filters=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not list()
req.list(filters) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

get_list() sounds a bit ugly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll change it right now. I'll also rename "get_filters" to just "filters" using the same logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds fine for me

@codecov-io
Copy link

Codecov Report

Merging #30 into master will decrease coverage by 0.01%.
The diff coverage is 95.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   93.79%   93.77%   -0.02%     
==========================================
  Files          28       28              
  Lines        1096     1093       -3     
==========================================
- Hits         1028     1025       -3     
  Misses         68       68
Impacted Files Coverage Δ
connect/resource/usage_automation.py 93.85% <100%> (ø) ⬆️
connect/resource/automation.py 92.3% <100%> (-0.8%) ⬇️
connect/resource/base.py 92.3% <100%> (ø) ⬆️
connect/resource/fulfillment_automation.py 52.83% <100%> (ø) ⬆️
connect/resource/template.py 60% <66.66%> (ø) ⬆️

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 c51c4c2...6b1ca31. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Apr 23, 2019

Codecov Report

Merging #30 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   93.79%   93.77%   -0.02%     
==========================================
  Files          28       28              
  Lines        1096     1093       -3     
==========================================
- Hits         1028     1025       -3     
  Misses         68       68
Impacted Files Coverage Δ
connect/resource/usage_automation.py 93.85% <100%> (ø) ⬆️
connect/resource/automation.py 92.3% <100%> (-0.8%) ⬇️
connect/resource/template.py 60% <100%> (ø) ⬆️
connect/resource/base.py 92.3% <100%> (ø) ⬆️
connect/resource/fulfillment_automation.py 52.83% <100%> (ø) ⬆️

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 c51c4c2...5a1a76e. Read the comment docs.

@vgrebenschikov vgrebenschikov merged commit d38a1e3 into cloudblue:master Apr 23, 2019
@JaviCerveraIngram JaviCerveraIngram deleted the CPS-10-get-list branch April 24, 2019 09:20
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.

5 participants