-
Notifications
You must be signed in to change notification settings - Fork 107
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
Content contained in RepoVer and Publication #1245
Conversation
b2f58a8
to
4544ba0
Compare
Attached issue: https://pulp.plan.io/issues/4832 |
4544ba0
to
3f93406
Compare
Some tests would be lovely 😺 |
1ec294a
to
1d3cf57
Compare
1d3cf57
to
ffb956d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggestion for a comment about ListRepositoryVersionViewSet
. Other then that, LGTM.
ffb956d
to
0a23f4e
Compare
@daviddavis comment about viewset added. |
@@ -504,6 +532,10 @@ def versions_containing_content(cls, content): | |||
Returns: | |||
django.db.models.QuerySet: Repository versions which contains content. | |||
""" | |||
deprecation_logger( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs a changelog entry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog entry extended by method goes deprecated message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes listed in CHANGES
are meant for end users. We need a changelog entry for plugin writers (in CHANGES/plugin_api
). Also, it should use the .deprecation
extension (CHANGES/plugin_api/4832.deprecation
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changelog divided to .feature and .deprecation
cf39f8f
to
dd79126
Compare
I just noticed that the tests don't use bindings. It's recommended new tests should use the bindings. |
e0830c8
to
93a48fb
Compare
tests are using bindings now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one superfluous method now. But my approval stands.
versions = list(itertools.chain.from_iterable(versions)) | ||
|
||
return qs.filter(number__in=versions) | ||
return qs.with_content([content.pk]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the doc block for RepositoryVersionContentFilter
is outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some basic testing it seems to do the job :)
93a48fb
to
50c44f8
Compare
Views to show repository_versions or publications which contains specific content. closes: pulp#4832 https://pulp.plan.io/issues/4832
50c44f8
to
7c7ed22
Compare
Views to show repository_versions or publications which contain specific content.
closes: #4832
https://pulp.plan.io/issues/4832