Skip to content

Fix CI and overrides#396

Merged
Alkarex merged 3 commits intomainfrom
fix-overrides
Nov 23, 2025
Merged

Fix CI and overrides#396
Alkarex merged 3 commits intomainfrom
fix-overrides

Conversation

@Alkarex
Copy link
Member

@Alkarex Alkarex commented Nov 23, 2025

#364 (comment)
Testing needed

declare(strict_types=1);

class FreshExtension_auth_Controller extends FreshRSS_auth_Controller {
class FreshExtension_authCaptcha_Controller extends FreshRSS_auth_Controller {
Copy link
Member

@Inverle Inverle Nov 23, 2025

Choose a reason for hiding this comment

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

Both the filename change and class name change make this extension no longer work properly (since this is now a new controller, instead of overriding auth).
This requires a further look for a working solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. I am afraid the approach of overriding classes like that only works with one extension at a time

Copy link
Member

Choose a reason for hiding this comment

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

So hooks would be needed in 2 places then, after all.
Or maybe it's possible to add support for overriding an action with multiple extensions at once into Minz.

@Inverle Inverle marked this pull request as draft November 23, 2025 20:19
@Alkarex Alkarex marked this pull request as ready for review November 23, 2025 20:40
@Alkarex Alkarex marked this pull request as draft November 23, 2025 21:10
@Alkarex Alkarex marked this pull request as ready for review November 23, 2025 21:43
@Alkarex Alkarex merged commit c744be2 into main Nov 23, 2025
1 check passed
@Alkarex Alkarex deleted the fix-overrides branch November 23, 2025 21:45
@Alkarex
Copy link
Member Author

Alkarex commented Nov 23, 2025

Help welcome to rethink the approach, preferably with extension hooks, or with a reworked overriding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants