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

New URLs builders for Downloader and Mediawiki classes #1854

Merged
merged 4 commits into from
Aug 1, 2023
Merged

New URLs builders for Downloader and Mediawiki classes #1854

merged 4 commits into from
Aug 1, 2023

Conversation

DonAlexandro
Copy link
Contributor

Overview

This PR will add URL builders as a part of #1830 architecture revamping

@codecov
Copy link

codecov bot commented Jun 13, 2023 •
edited
Loading

Codecov Report

Patch coverage: 95.20% and project coverage change: +0.55% 🎉

Comparison is base (e99aa3a) 69.73% compared to head (24b2564) 70.29%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1854      +/-   ##
==========================================
+ Coverage   69.73%   70.29%   +0.55%     
==========================================
  Files          24       31       +7     
  Lines        2647     2720      +73     
  Branches      601      599       -2     
==========================================
+ Hits         1846     1912      +66     
- Misses        690      696       +6     
- Partials      111      112       +1
Files Changed Coverage Δ
src/Downloader.ts 61.96% <86.66%> (-0.62%) ⬇️
src/util/builders/url/api.director.ts 89.47% <89.47%> (ø)
src/MediaWiki.ts 80.88% <91.66%> (-0.82%) ⬇️
src/util/builders/url/url.builder.ts 97.43% <97.43%> (ø)
src/Dump.ts 88.23% <100.00%> (+0.11%) ⬆️
src/mwoffliner.lib.ts 72.37% <100.00%> (+0.10%) ⬆️
src/util/builders/url/base.director.ts 100.00% <100.00%> (ø)
src/util/builders/url/basic.director.ts 100.00% <100.00%> (ø)
src/util/builders/url/desktop.director.ts 100.00% <100.00%> (ø)
src/util/builders/url/visual-editor.director.ts 100.00% <100.00%> (ø)
... and 2 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kelson42
Copy link
Collaborator

@DonAlexandro Thank you for having isolated this in a PR. @VadimKovalenkoSNF Will make the code review at code level and polish a few things to make it ready to merge.

VadimKovalenkoSNF
VadimKovalenkoSNF reviewed Jul 18, 2023
View reviewed changes
src/util/builders/url/base.director.ts Outdated
buildMobileRestApiURL(path?: string) {
return urlBuilder
.setDomain(this.baseDomain)
.setPath(path ?? 'api/rest_v1/page/mobile-sections')
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kelson42 I'm going to remove all methods related to /mobile-sections endpoint because it has been disabled for a while, you can check the doc - https://en.wikipedia.org/api/rest_v1/#/Mobile/getSections. Pls. let me know, if you have any concerns about it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@VadimKovalenkoSNF Just please articulate/organize your moves (remove mobile-section support, add support of new mobile HTML end-point) with #1664 (future PR)

@kelson42 kelson42 mentioned this pull request Jul 18, 2023
Closed
@VadimKovalenkoSNF VadimKovalenkoSNF force-pushed the feature/1830-rearchitecturing/urls-builders branch 2 times, most recently from 9db362a to 4d6aa81 Compare July 19, 2023 13:23
@VadimKovalenkoSNF
Copy link
Collaborator

CI based on #1862. PR1862 should be merged first.

@VadimKovalenkoSNF VadimKovalenkoSNF force-pushed the feature/1830-rearchitecturing/urls-builders branch from 4d6aa81 to d33c197 Compare July 28, 2023 11:32
@VadimKovalenkoSNF
Copy link
Collaborator

CI based on #1862. PR1862 should be merged first.

Done.

@VadimKovalenkoSNF VadimKovalenkoSNF mentioned this pull request Aug 1, 2023
Merged
@kelson42 kelson42 changed the title [Feature] URLs builders for Downloader and Mediawiki New URLs builders for Downloader and Mediawiki classes Aug 1, 2023
@kelson42 kelson42 force-pushed the feature/1830-rearchitecturing/urls-builders branch from d33c197 to 24b2564 Compare August 1, 2023 09:47
kelson42
kelson42 approved these changes Aug 1, 2023
View reviewed changes
Copy link
Collaborator

@kelson42 kelson42 left a comment

Choose a reason for hiding this comment

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

LGTM

@kelson42 kelson42 merged commit 1cd6963 into main Aug 1, 2023
5 checks passed
@kelson42 kelson42 deleted the feature/1830-rearchitecturing/urls-builders branch August 1, 2023 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kelson42 kelson42 kelson42 approved these changes

@VadimKovalenkoSNF VadimKovalenkoSNF Awaiting requested review from VadimKovalenkoSNF

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DonAlexandro @kelson42 @VadimKovalenkoSNF