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

QuerySets: refactor _add_user_repos #8182

Merged
merged 4 commits into from May 25, 2021

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented May 14, 2021

Get ready for sharing more code,
mainly rename _add_user_repos to _add_from_user_projects and
_add_user_projects.

Start accepting admin and member attributes where needed,
these aren't used here yet, but they will.

I have removed some overrides that weren't needed or weren't overridden
in .com.

And for my next trick/PR, I'd try to move some overrides into .org :D

Get ready for sharing more code,
mainly rename `_add_user_repos` to `_add_from_user_projects` and
`_add_user_projects`.

Stat accepting admin and member attributes,
this aren't used here yet, but they will.

I have removed some overrides that weren't needed or weren't overridden
in .com.
@stsewd stsewd requested a review from a team May 14, 2021 01:26
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

I haven't tested this heavily, but the fact that tests didn't break seems positive :)

.. note::

In .org all users are admin and member of a project.
This will change with organizations soon.
Copy link
Member

Choose a reason for hiding this comment

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

Solid comment 👍

readthedocs/builds/querysets.py Outdated Show resolved Hide resolved
@@ -79,18 +79,6 @@ def test_subproject_queryset_attributes(self):
self.assertEqual(ChildRelatedProjectQuerySet.project_field, 'child')
self.assertTrue(ChildRelatedProjectQuerySet.use_for_related_fields)

def test_subproject_queryset_as_manager_gets_correct_class(self):
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to delete this? We don't want to check that the subproject still gets the right class, just without the Base?

Copy link
Member Author

Choose a reason for hiding this comment

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

Test is back!

stsewd and others added 2 commits May 24, 2021 19:05
Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>
@stsewd stsewd enabled auto-merge (squash) May 25, 2021 00:16
@stsewd stsewd merged commit 0c81b04 into master May 25, 2021
@stsewd stsewd deleted the querysets-get-ready-for-sharing-more branch May 25, 2021 00:17
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

2 participants