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

Update ImageProxy code #270

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Update ImageProxy code #270

merged 4 commits into from
Jan 13, 2025

Conversation

Alkarex
Copy link
Member

@Alkarex Alkarex commented Jan 12, 2025

fix #202
Note: breaking changes. Users will have to save their configuration again

fix #202
Note: breaking changes. Users will have to save their configuration again
@Alkarex Alkarex requested a review from Frenzie January 12, 2025 23:10
@Alkarex
Copy link
Member Author

Alkarex commented Jan 12, 2025

Not tested.

@Frenzie You are welcome to check if you want to add some auto migration of the former way of storing user preferences (storing at user config top level was never meant to be supported).

@Frenzie
Copy link
Member

Frenzie commented Jan 13, 2025

The settings are extremely basic so that doesn't seem very necessary but then again there should probably be some kind of warning about it.

@Alkarex
Copy link
Member Author

Alkarex commented Jan 13, 2025

The settings are extremely basic so that doesn't seem very necessary but then again there should probably be some kind of warning about it.

I am not sure were the warnings should be given, though

@Frenzie
Copy link
Member

Frenzie commented Jan 13, 2025

The README seems to have acquired a changelog. Maybe that suffices?
https://github.com/FreshRSS/Extensions/tree/c726958f3fe474c013237c8d885a03225121933c/xExtension-ImageProxy#changelog

There is one other fairly obvious place, but presumably that would require development.
image

@Alkarex
Copy link
Member Author

Alkarex commented Jan 13, 2025

Would you be able to test that it still work as it should?

@Frenzie
Copy link
Member

Frenzie commented Jan 13, 2025

The proxy HTTP setting works, but the proxy HTTPS setting doesn't seem to.

@Alkarex
Copy link
Member Author

Alkarex commented Jan 13, 2025

Please try again

@Frenzie
Copy link
Member

Frenzie commented Jan 13, 2025

I don't have anything at hand to test the "proxy unspecified" setting (that should probably be changed to "Proxy unspecified scheme" or something because I had no idea what it was about anymore without looking at the code) but the rest seems to work now. 👍

@Alkarex Alkarex merged commit 4dd0450 into master Jan 13, 2025
1 check passed
@Alkarex Alkarex deleted the update-image-proxy branch January 13, 2025 22:34
@Alkarex
Copy link
Member Author

Alkarex commented Jan 13, 2025

Super. Let's made those changes in another PR. Anyone?

Frenzie added a commit to Frenzie/Extensions that referenced this pull request Jan 14, 2025
Alkarex added a commit that referenced this pull request Jan 15, 2025
* ImageProxy: update text for "Update unspecified scheme"

See <#270 (comment)>.

* URL scheme

* add Dutch

* Update xExtension-ImageProxy/i18n/fr/ext.php

Co-authored-by: Alexandre Alapetite <[email protected]>

* Turkish

* protocol-relative URL

* Update xExtension-ImageProxy/i18n/fr/ext.php

Co-authored-by: Alexandre Alapetite <[email protected]>

* fix typo

* Update xExtension-ImageProxy/i18n/nl/ext.php

Co-authored-by: Alexandre Alapetite <[email protected]>

---------

Co-authored-by: Alexandre Alapetite <[email protected]>
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.

Update ImageProxy to pass PHPStan Level 9
2 participants