Combining role and role version into a single content unit #91
Conversation
|
Hello @daviddavis! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-04-17 15:52:39 UTC |
daviddavis
force-pushed
the
issue3818
branch
4 times, most recently
from
864a359
to
019affb
Compare
|
I went through the entire readme and everything worked for me! It also makes this conceptually simpler for users. |
| @@ -10,30 +10,14 @@ | |||
|
|
|||
| class AnsibleRole(Content): | |||
| """ | |||
| A model representing an Ansible Role. | |||
| A content type representing an Ansible Role version. | |||
| """ | |||
|
|
|||
| TYPE = 'ansible-role' | |||
There was a problem hiding this comment.
This causes the API serializer to have "_type": "ansible.ansible-role", What about renaming AnsibleRole to Role throughout and this to 'role'?
There was a problem hiding this comment.
We can handle this separately. I wrote this issue and put it onto the 0.1 release. https://pulp.plan.io/issues/4709
There was a problem hiding this comment.
This PR works great @daviddavis thank you! I think this is a big step in the right direction.
https://pulp.plan.io/issues/3818
fixes #3818