Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

refactor(v4): split objects and managers per API resource#1288

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
max-wittig merged 2 commits intomasterfromrefactor/split-objects
Feb 15, 2021

Conversation

nejch
Copy link
Member

@nejchnejch commentedFeb 7, 2021•
edited
Loading

This one is huge, so sorry 🙈 It does what#997 was for (splitting the 6000 line monster), but with less restructuring (only splitting the resources into modules). I'm not sure if all the importing is the best approach, but it's quite explicit.

I can imagine this is too much for review so I can try to split it, but the managers were quite intertwined so I was fixing things along the way. Here is the new import chain (mostly due to Project and Group including many other managers) - output ofpydeps --rmprefix='gitlab.v4.objects.' --only='gitlab.v4.objects' gitlab/v4/objects/__init__.py:

image

Each module only imports the managers it needs, while__init__.py does a star import of all of them for backwards compatibility. Each module exports only its own classes for the star import explicitly via__all__.

In the end this needed to happen in my opinion as well, to keep sane file sizes. But not sure what's more painful, reviewing this big PR or doing it in stages. I'm sure there are better ways to do this so if anyone has ideas, while keeping backwards compatibility, I'd be open too! :)

If this gets merged I can rebase other people's PR's touchingobjects so it's not too demotivating for them.

This would now roughly imitate the structure of the GitLab API docs and gitlab's ownlib/api directory:

max-wittig reacted with laugh emojimax-wittig reacted with rocket emojimax-wittig and alberand reacted with eyes emoji
@nejch
Copy link
MemberAuthor

nejch commentedFeb 7, 2021•
edited
Loading

Oh no 😁
image

"Ready for review"... 🤣

@nejchnejch marked this pull request as ready for reviewFebruary 7, 2021 21:51
@max-wittig
Copy link
Member

That's a nice MR 👍 Will indeed take some time to review 😄

@nejch
Copy link
MemberAuthor

That's a nice MR 👍 Will indeed take some time to review 😄

Understandable 😄 I found that withgit diff --color-moved master, it should show with different colors what's moved and what's added/deleted (orgit diff --color-moved=plain master since the default gives you a funky zebra pattern).

So it'd show it's mostly just cut/paste :) I couldn't really find a good GUI difftool that does it well for moves across files but I'm sure there's some out there, if it makes it a bit clearer.

@max-wittig
Copy link
Member

Thanks for all the refactoring done here 👍

@max-wittigmax-wittig merged commit9fcd962 intomasterFeb 15, 2021
@max-wittigmax-wittig deleted the refactor/split-objects branchFebruary 15, 2021 07:48
@bufferoverflow
Copy link
Member

@nejch very nice work!@max-wittig a heavy review 😄

@JohnVillalovos
Copy link
Member

@nejch Any plans on doing something similar for gitlab/init.py ?

@nejch
Copy link
MemberAuthor

nejch commentedFeb 15, 2021•
edited
Loading

@nejch Any plans on doing something similar for gitlab/init.py ?

I wasn't currently working on anything, but maybe it would make sense to take the class out into something like aclient module to make it slim, and possibly split it a bit or if you have any other ideas. I think@max-wittig you also mentioned you wanted some cleanup there some time ago? 😀 But maybe you can open a new issue (or PR) for that.

@JohnVillalovos
Copy link
Member

@nejch Any plans on doing something similar for gitlab/init.py ?

I wasn't currently working on anything, but maybe it would make sense to take the class out into something like aclient module to make it slim, and possibly split it a bit or if you have any other ideas. I think@max-wittig you also mentioned you wanted some cleanup there some time ago? 😀 But maybe you can open a new issue (or PR) for that.

Thanks. I'll try to do a PR and move most of what is ingitlab/__init__.py togitlab/client.py.

@nejchnejch mentioned this pull requestFeb 24, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@max-wittigmax-wittigAwaiting requested review from max-wittig

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@nejch@max-wittig@bufferoverflow@JohnVillalovos
[8]ページ先頭
©2009-2025 Movatter.jp