Skip to content

Clean up RSS14Reader#631

Merged
werthdavid merged 4 commits into
zxing-js:feature/modernize26from
opl-:feat/clean-up-rss14reader
Apr 22, 2026
Merged

Clean up RSS14Reader#631
werthdavid merged 4 commits into
zxing-js:feature/modernize26from
opl-:feat/clean-up-rss14reader

Conversation

@opl-

@opl- opl- commented Nov 23, 2024

Copy link
Copy Markdown
Contributor

Ported zxing/zxing#1179, changed a bunch of let to const where appropriate, copied over a couple comments, initialized some variables, changed == to ===, and fixed thrown errors getting eaten in one place.

@opl-
opl- force-pushed the feat/clean-up-rss14reader branch from 755a2dc to 2fc10e6 Compare November 25, 2024 12:17
@werthdavid

Copy link
Copy Markdown
Member

Thank you for all the PRs, much appreciated!!! I will take a look after my vacation! 👍🏻

Comment thread src/core/oned/rss/RSS14Reader.ts
@werthdavid

Copy link
Copy Markdown
Member

Can you rebase that and resolve the conflicts?

@opl-
opl- force-pushed the feat/clean-up-rss14reader branch from 2fc10e6 to 724d31f Compare December 3, 2024 14:35
@werthdavid

Copy link
Copy Markdown
Member

some tests are still failing @opl-

@opl-

opl- commented Mar 22, 2026

Copy link
Copy Markdown
Contributor Author

It's been over a year and the build logs expired, but I assume the reason for the failing tests was the same as with #630 - the image manipulation library used for testing being unable to rotate or otherwise transform the test images?

From what I recall the point of this PR was to fix the thrown errors getting eaten, and I happened to do some clean up and port a couple commits from the original library while at it (split into separate commits). Was never my intention to make all tests pass, so as long as this doesn't make more tests fail, I don't see a reason not to merge this.

@werthdavid
werthdavid changed the base branch from master to feature/modernize26 April 22, 2026 20:43
@werthdavid
werthdavid merged commit c10ad76 into zxing-js:feature/modernize26 Apr 22, 2026
2 of 3 checks passed
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