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

[PixivBridge] Add User context #2650

Merged
merged 1 commit into from
May 8, 2022
Merged

[PixivBridge] Add User context #2650

merged 1 commit into from
May 8, 2022

Conversation

yamanq
Copy link
Contributor

@yamanq yamanq commented Apr 13, 2022

This is almost a rewrite of the bridge so that it better handles multiple contexts. One important note is that the maximum number of posts returned is now decreased if selecting a specific Post Type, but this limit is still 24 for illustrations/manga and 8 for novels, which I think is high enough for almost all use cases. I also added a new Post Type, "All" which aggregates results from all types.

Closes #2635

@github-actions
Copy link

Pull request artifacts

file last change
Pixiv-current-context1 2022-04-13, 00:34:40
Pixiv-pr-context1 2022-04-13, 00:34:40
Pixiv-pr-context2 2022-04-13, 00:34:40

@yamanq
Copy link
Contributor Author

yamanq commented Apr 13, 2022

I'm not sure how to maintain backwards compatibility while allowing the tests to pass.

@dvikan
Copy link
Contributor

dvikan commented May 8, 2022

I tested with an existing feed url and this PR does not break old urls. Thats good.

@dvikan dvikan merged commit 410daee into RSS-Bridge:master May 8, 2022
@yamanq
Copy link
Contributor Author

yamanq commented May 8, 2022

Yes, I coded it to be backwards compatible. However, this makes the tests (specifically the linting tests) fail.

@dvikan
Copy link
Contributor

dvikan commented May 8, 2022

There is a unit test that disallows context to be the empty string. I don't see a good reason for that. And in this case we actually do want that. I don't understand how it preserves BC, but it does. I suggest remove test.

EDIT: I excluded Pixiv for this particular assertion.

Kwbmm pushed a commit to Kwbmm/rss-bridge that referenced this pull request Jun 17, 2022
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.

Bridge request for Pixiv User
2 participants