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

Introduce Criteria.with_unit_type #112

Merged
merged 1 commit into from Sep 1, 2021

Conversation

rohanpm
Copy link
Member

@rohanpm rohanpm commented Aug 24, 2021

It was already possible for callers to search for units of a specific
type by doing, for example:

Criteria.with_field("content_type_id", Matcher.in_(["rpm", "srpm"]))

The problem with this is that it requires the caller to know which
values of content_type_id map to each model class such as RpmUnit.
The library already knows this implementation detail internally and it's
best to avoid duplicating it also in the user of the library.

This commit adds a helper which accepts Unit classes as input, as in:

Criteria.with_unit_type(RpmUnit)

This allows the mapping between unit subclasses and content types to
remain fully encapsulated within this library.

@rohanpm rohanpm force-pushed the with-unit-type branch 2 times, most recently from 9633a76 to fd41a8d Compare August 29, 2021 22:22
It was already possible for callers to search for units of a specific
type by doing, for example:

  Criteria.with_field("content_type_id", Matcher.in_(["rpm", "srpm"]))

The problem with this is that it requires the caller to know which
values of content_type_id map to each model class such as RpmUnit.
The library already knows this implementation detail internally and it's
best to avoid duplicating it also in the user of the library.

This commit adds a helper which accepts Unit classes as input, as in:

  Criteria.with_unit_type(RpmUnit)

This allows the mapping between unit subclasses and content types to
remain fully encapsulated within this library.
@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #112 (f6eebf3) into master (36082ba) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #112   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         1742      1755   +13     
=========================================
+ Hits          1742      1755   +13     
Impacted Files Coverage Δ
pubtools/pulplib/_impl/client/search.py 100.00% <ø> (ø)
pubtools/pulplib/_impl/criteria.py 100.00% <100.00%> (ø)
pubtools/pulplib/_impl/model/unit/__init__.py 100.00% <100.00%> (ø)
pubtools/pulplib/_impl/model/unit/base.py 100.00% <100.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 36082ba...f6eebf3. Read the comment docs.

@rohanpm rohanpm marked this pull request as ready for review August 30, 2021 22:12
@rohanpm rohanpm requested a review from a team August 30, 2021 22:12
@rohanpm rohanpm merged commit 13bcba0 into release-engineering:master Sep 1, 2021
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