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.

Sign up for GitHub

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

Jump to bottom

JikanPy v4 #72

Merged
merged 14 commits into from
May 21, 2020
Merged

JikanPy v4 #72

merged 14 commits into from
May 21, 2020

Conversation

abhinavk99
Copy link
Owner

Fixes #68, #69, #70, and #71

@abhinavk99 abhinavk99 self-assigned this May 21, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 21, 2020 •
edited
Loading

Codecov Report

Merging #72 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            master       #72    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            6         5     -1     
  Lines          355       245   -110     
==========================================
- Hits           355       245   -110
Impacted Files Coverage Δ
jikanpy/exceptions.py 100.00% <ø> (ø)
jikanpy/aiojikan.py 100.00% <100.00%> (ø)
jikanpy/jikan.py 100.00% <100.00%> (ø)
jikanpy/utils.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b041671...301e25e. Read the comment docs.

seanbreckenridge
seanbreckenridge approved these changes May 21, 2020
View reviewed changes
Copy link
Collaborator

@seanbreckenridge seanbreckenridge left a comment

Choose a reason for hiding this comment

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

everything looks good, I added a link to the docs in the usage. would be easier for me to contribute once the docs are up, thats where I'd be most interested in improving. Only other thing I could suggest adding a CONTRIBUTING.md file which describes how to work on the documentation locally.

abhinavk99 and others added 8 commits May 20, 2020 21:19
@abhinavk99
Construct aiohttp session at the start of async methods (fixes #70)
854dc0f 
Construct aiohttp session at the start of async methods (fixes #70)

Changed aiojikan to get session in each method to fix mypy errors
@abhinavk99
Replaced AbstractJikan with utility methods and removed unnecessary t…
582e1a5 
…ype hints (fixes #71)
@abhinavk99
Added optional page arg to club method
7bcf82a
@abhinavk99
Fixed adding page to user URL when there's no argument
5046c82
@abhinavk99
Rewrote docstrings to Google style, generated HTML docs using sphinx …
707214b 
…and readthedocs (fixes #69)

Rewrote docstrings to Google style and generated HTML docs using sphinx (fixes #69)

Set sphinx master doc to index

Added readthedocs configuration yaml

Pointed readthedocs install requirements to requirements.txt

Added link to docs
@abhinavk99
Added test for constructing aiojikan using async with
7ccbac7
@seanbreckenridge @abhinavk99
add link to documentation below
ac38556
@abhinavk99
Updated user method docstrings and readthedocs config yaml
2369afc
@abhinavk99
Copy link
Owner Author

The docs are actually currently at https://jikanpy.readthedocs.io/en/v4/. Once I merge this in, https://jikanpy.readthedocs.io/en/latest will have it as it tracks the master branch. Working on documentation is just editing docstrings.

@abhinavk99 abhinavk99 merged commit c65a488 into master May 21, 2020
@abhinavk99 abhinavk99 deleted the v4 branch May 21, 2020 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanbreckenridge seanbreckenridge seanbreckenridge approved these changes

Assignees

@abhinavk99 abhinavk99

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove parameter checks
3 participants
@abhinavk99 @codecov-commenter @seanbreckenridge