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

fix null coalescent operator expression #518

Merged
merged 1 commit into from
Mar 31, 2022
Merged

fix null coalescent operator expression #518

merged 1 commit into from
Mar 31, 2022

Conversation

mjauvin
Copy link
Member

@mjauvin mjauvin commented Mar 30, 2022

  • add parenthesis around expression, was breaking when compiling the js asset without this
  • recompile all js assets

@mjauvin mjauvin added maintenance PRs that fix bugs, are translation changes or make only minor changes needs review Issues/PRs that require a review from a maintainer labels Mar 30, 2022
@mjauvin mjauvin added this to the v1.2.0 milestone Mar 30, 2022
@mjauvin mjauvin self-assigned this Mar 30, 2022
bennothommo
bennothommo approved these changes Mar 31, 2022
View reviewed changes
Copy link
Member

@bennothommo bennothommo left a comment

Choose a reason for hiding this comment

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

Looks fine to me, if it's all tested and works, then feel free to merge.

@mjauvin you're right, null coalesce is definitely a valid operator, so the problem is with the asset compiler. I don't think the same problem would occur with the new Mix compiler.

@LukeTowers
Copy link
Member

This might be worth reporting to wikimedia (I believe the JS minifier is now using https://github.com/wikimedia/minify).

@mjauvin mjauvin added Status: Completed and removed needs review Issues/PRs that require a review from a maintainer labels Mar 31, 2022
@mjauvin mjauvin merged commit 752d6ad into wintercms:wip/1.2 Mar 31, 2022
@mjauvin mjauvin deleted the fix-js-filter branch March 31, 2022 11:41
@LukeTowers
Copy link
Member

@mjauvin are you able to report the issue to wikimedia?

@mjauvin
Copy link
Member Author

mjauvin commented Apr 1, 2022

@LukeTowers I tried to open an issue, but couldn't find a way without creating a developper account.

Do you have such an account yourself?

@LukeTowers
Copy link
Member

No, you'd have to create one.

@mjauvin
Copy link
Member Author

mjauvin commented Apr 1, 2022

Man, forget it. Ever heard of "La maison des fous" ? You get bounced all over the place and never end-up in the right place... this is MADENING.

Why can't they have github issue enabled and that's it?

@mjauvin
Copy link
Member Author

mjauvin commented Apr 1, 2022

Issue opened at wikimedia:
https://phabricator.wikimedia.org/T305277

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

@bennothommo bennothommo bennothommo approved these changes

@LukeTowers LukeTowers Awaiting requested review from LukeTowers

Assignees

@mjauvin mjauvin

Labels
maintenance PRs that fix bugs, are translation changes or make only minor changes
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@mjauvin @LukeTowers @bennothommo