-
Notifications
You must be signed in to change notification settings - Fork 671
feat(api): add support for wiki attachments #2722
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
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2722 +/- ##
=======================================
Coverage 96.44% 96.45%
=======================================
Files 88 88
Lines 5761 5775 +14
=======================================
+ Hits 5556 5570 +14
Misses 205 205
Flags with carried forward coverage won't be shown. Click here to find out 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.
Thanks a lot @TigreModerata, I like the direction here but just had a question about the exception raising. This would also potentially address the coverage drop I think. Let me know what you think!
I removed the exception. Changed _upload_path to _upload_string in Project, ProjectWiki and UploadMixin. If you are happy with the overall approach, maybe I should add the tests for the mixin and other unit tests (and update the docs)? |
Sorry again about the commit messages, I'll fix it in the next one, have to run! |
My suggestion was actually in the other direction (to use |
@TigreModerata no worries, it's best to just squash the commits into one when you're finished with the fixups. |
dee8663
to
f076d7e
Compare
OK, I managed an ok commit :) |
Added UploadMixin in mixin module Added UplaodMixin dependency for Project, ProjectWiki, GroupWiki Added api tests for wiki upload Added unit test for mixin Added docs sections to wikis.rst
f076d7e
to
fce6896
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.
Thanks again @TigreModerata, I had a few more comments here, let me know what you think!
Added UploadMixin in mixin module Added UplaodMixin dependency for Project, ProjectWiki, GroupWiki Added api tests for wiki upload Added unit test for mixin Added docs sections to wikis.rst
21a52ed
to
4e7a881
Compare
Separated unit tests for upload mixin and re-added the file upload unit tests for projects |
Thanks a lot for your patience @TigreModerata! I squashed the commits in the end as there were 2 identical commit messages 🙇 |
OK, l have no idea what happened (I did many wrong resets, to squash commits, and I never hit the right point it seems) - I will learn one day. Thank YOU for your patience, are you kidding! I do realize that my "help" is more of a chore, but you guys' help and patience is the only way I make any progress at all! |
No worries @TigreModerata reviewing can be easier than starting things sometimes 😅 Most open issues could use some more love but some are definitely lower-hanging fruit - I'd say things that are labelled https://github.com/python-gitlab/python-gitlab/issues?q=is%3Aissue+is%3Aopen+label%3Afeature |
Changes
UploadMixin added, inherits from RESTObject;
UploadMixin used in projects.py and wikis.py to upload files;
Test for upload to wiki api added in tests/functional/api/test_wikis.
Documentation and unit tests not yet added - waiting for corrections/objections
Documentation and testing
Please consider whether this PR needs documentation and tests. This is not required, but highly appreciated: