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

Switches SFS to a fork fixing ParserOptions constructor call error #262

Closed
wants to merge 1 commit into from

Conversation

vedmaka
Copy link
Collaborator

@vedmaka vedmaka commented Jul 16, 2023

Uses forked version to include this PR SemanticMediaWiki/SemanticFormsSelect#104 to fix ParserOptions constructor call missing UserIdentity parameter (https://phabricator.wikimedia.org/T284977)

Uses forked version to include this PR SemanticMediaWiki/SemanticFormsSelect#104 to fix ParserOptions constructor call missing UserIdentity parameter
@github-actions
Copy link

🐳 The image based on dbb9995b commit has been built with 1.39.1-20230716-262 tag as ghcr.io/canastawiki/canasta:1.39.1-20230716-262

@yaronkoren
Copy link
Member

This indeed looks like a problem. I have some questions:

  • Has Semantic Forms Select never worked with MW 1.37 or higher? (If so, that probably says a lot about SFS's popularity, and possibly about its usefulness too - but that's a separate conversation.)
  • Is the protocol now to switch to a forked version as soon as some bug is discovered? This does seem like a highest-severity bug, but on the other hand, the patch to fix it was only submitted a few hours ago. For now, can't you just fix this line of code locally, on the relevant wiki?
  • Speaking of the patch, I don't actually see the fix in the forked version you want to switch to - https://github.com/vedmaka/SemanticFormsSelect - which seems to have been last updated in 2019. (!) What's going on there?

@vedmaka
Copy link
Collaborator Author

vedmaka commented Jul 16, 2023

Has Semantic Forms Select never worked with MW 1.37 or higher? (If so, that probably says a lot about SFS's popularity, and possibly about its usefulness too - but that's a separate conversation.)

Apparently

Is the protocol now to switch to a forked version as soon as some bug is discovered? This does seem like a highest-severity bug, but on the other hand, the patch to fix it was only submitted a few hours ago. For now, can't you just fix this line of code locally, on the relevant wiki?

I am not quite sure how to treat the severity of this, but the bug renders the extension dysfunctional, and due to how SFS is composed #248 (comment) there is not much sense in trying to fix this on some particular wiki, at the stack level

Speaking of the patch, I don't actually see the fix in the forked version you want to switch to - https://github.com/vedmaka/SemanticFormsSelect - which seems to have been last updated in 2019. (!) What's going on there?

Please note that the PR is created from the patch branch https://github.com/vedmaka/SemanticFormsSelect/tree/patch-2

@yaronkoren
Copy link
Member

Okay, thank you for the explanations. Sorry for pressing the point, but I just want to make sure I understand this: can't you just go in to the local SFS code and change that one line? Of course, your change will get wiped out as soon as you refresh Canasta, but if the issue is just getting the code to work in the next few hours, that would seem like a good-enough solution.

Beyond this, I really would like to have a discussion, whether here or elsewhere, about what the useful parts are of SFS - and how they can be replicated in Page Forms or elsewhere. The fact that SFS has apparently been broken for two years would seem to suggest that it should be avoided if at all possible.

@vedmaka
Copy link
Collaborator Author

vedmaka commented Jul 16, 2023

can't you just go in to the local SFS code and change that one line? Of course, your change will get wiped out as soon as you refresh Canasta, but if the issue is just getting the code to work in the next few hours, that would seem like a good-enough solution.

This would break the rule of not making changes to a running container ever :) Seriously, no one should be doing such a horrible thing ever!

Speaking of a local patch - yes, that's doable with some extra efforts, but I really doubt we will get a fix to SFS merged soon, and thus it's better to have a Canasta image with a working extension rather than rolling back patches across all stacks affected

The good news is that it's unnecessary to merge this commit immediately. All I need for now is the image that was built to continue migrating a wiki to Canasta 1.39

Beyond this, I really would like to have a discussion, whether here or elsewhere, about what the useful parts are of SFS - and how they can be replicated in Page Forms or elsewhere. The fact that SFS has apparently been broken for two years would seem to suggest that it should be avoided if at all possible.

I personally don't like the SFS, and if it's possible to rework the client wiki code to replace SFS with anything - I'd vote for it. But this is not a decision I can make by myself

@yaronkoren
Copy link
Member

I really doubt we will get a fix to SFS merged soon

What makes you think that?

@tosfos
Copy link
Collaborator

tosfos commented Jul 17, 2023

It's been merged 😄 SemanticMediaWiki/SemanticFormsSelect#104

@vedmaka
Copy link
Collaborator Author

vedmaka commented Jul 21, 2023

Closed in favor of #265

@vedmaka vedmaka closed this Jul 21, 2023
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.

3 participants