Skip to content

refactor(client): add _url & _ajax_url helper methods#384

Open
Danipulok wants to merge 5 commits into
upbit:masterfrom
Danipulok:refactor(url)
Open

refactor(client): add _url & _ajax_url helper methods#384
Danipulok wants to merge 5 commits into
upbit:masterfrom
Danipulok:refactor(url)

Conversation

@Danipulok

Copy link
Copy Markdown
Contributor

What this PR does?

  • Add _url and _ajax_url helper methods to the class to omit boilerplate formatting;

TODO:

After this PR is merged (I hope), I would like to make all constants actual constants. It will allow us to reuse code and have it in one place even more

@upbit

upbit commented Jan 14, 2025

Copy link
Copy Markdown
Owner

There are significant differences in the interfaces and fields between www.pixiv.net and Pixiv App.
Pixivpy is designed based on the App's API, and currently, there are no plans for migrating to Web API, or using it in a mixed way.

@upbit upbit added the wontfix label Jan 14, 2025
@Danipulok

Copy link
Copy Markdown
Contributor Author

Got it, thanks for claryfying. There's already a method that uses Web API (showcase). So the idea is that if it's already used (I did not add it) is to make it kinda more generic

What about the methods like comments from my other PR that returns 404 now? And that method with showcase, which uses Web API and already in the class anyway...

@Danipulok

Copy link
Copy Markdown
Contributor Author

@upbit please tell me what you think about adding just _url then?
IMHO it's useful to format the url in a single place
Or should I just close this PR?

# Conflicts:
#	pixivpy3/aapi.py
@upbit

upbit commented Jan 14, 2025

Copy link
Copy Markdown
Owner

@upbit please tell me what you think about adding just _url then? IMHO it's useful to format the url in a single place Or should I just close this PR?

Both def _url(self, endpoint: str) -> str: and const hostname are good, avoiding the problem of modifying string magic numbers everywhere.

@upbit upbit removed the wontfix label Jan 15, 2025
@Danipulok

Danipulok commented Jan 15, 2025

Copy link
Copy Markdown
Contributor Author

@upbit I have removed the _ajax_url method. I was planning to do consts in another commit sooner. In a separate commits for all consts

# Conflicts:
#	pixivpy3/aapi.py
@Danipulok

Copy link
Copy Markdown
Contributor Author

@upbit I have updated the PR. Please review

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.

2 participants