-
Notifications
You must be signed in to change notification settings - Fork 20
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
Implement method for mobile-html #17
Conversation
Had previously been calling .json() on all responses, even if they were strings. Can now handle JSON and string responses. resolving dopecodez#14
Returns HTML for mobile for a given page. resolving dopecodez#14
I made an attempt at this issue but ran into some issues with |
I checked out the other functions of your forks quickly and it looks like everything is working. I'll check out the Feel free to change this from a draft when you are good with the changes. |
Thanks for the feedback. I just added |
I renamed the test file to match I also added mobileHtml to the page and added a few tests. |
There's actually no error thrown if the page isn't found. Wikipedia returns the below object, so I just passed it to the response. Should I instead do something like
|
No @friendofdog , i think its fine for now every We'll let it handle other errors and the |
@dopecodez understood. This PR is okay to merge now? |
Thanks for the PR @friendofdog , your work keeps this repo moving. We should definitely do something about the coverage failing like discussed in https://github.community/t/make-secrets-available-to-builds-of-forks/16166. I'll open a separate issue for this as this makes PRs look bad. |
Takes a page title and returns mobile-optimised HTML.
closes #14