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

DEV: replaces unecessary (action (mut .*)) by (mut .*) #10822

Merged
merged 2 commits into from
Oct 6, 2020
Merged

DEV: replaces unecessary (action (mut .*)) by (mut .*) #10822

merged 2 commits into from
Oct 6, 2020

Conversation

jjaffeux
Copy link
Contributor

@jjaffeux jjaffeux commented Oct 5, 2020

No description provided.

eviltrout
eviltrout reviewed Oct 5, 2020
View reviewed changes
Copy link
Contributor

@eviltrout eviltrout left a comment

Choose a reason for hiding this comment

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

I like it but it looks like there are failures.

@jjaffeux
Copy link
Contributor Author

jjaffeux commented Oct 6, 2020

@eviltrout should be good now

@CvX
Copy link
Contributor

CvX commented Oct 6, 2020

@jjaffeux do you know why fn is required in some cases?

@jjaffeux
Copy link
Contributor Author

jjaffeux commented Oct 6, 2020

@jjaffeux do you know why fn is required in some cases?

Yes it's generally due to this kind of cases or similar: https://github.com/discourse/discourse/blob/master/app/assets/javascripts/select-kit/addon/components/icon-picker.js#L96

What happens here is that mut gives us a mutable cell, not a function, so we can't use it as a function, I could do some type checking or something... but I think this is fine like this, also I should get rid of these legacy hacks some day.

@jjaffeux jjaffeux merged commit c0350dc into discourse:master Oct 6, 2020
@jjaffeux jjaffeux deleted the better-mut branch October 6, 2020 15:17
jjaffeux added a commit to jjaffeux/discourse that referenced this pull request Oct 6, 2020
@jjaffeux
Revert "DEV: replaces unecessary (action (mut .*)) by (mut .*) (disco…
b9a92ab 
…urse#10822)"

This reverts commit c0350dc.
jjaffeux added a commit that referenced this pull request Oct 6, 2020
@jjaffeux
REVERT: revert change from action + mut to mut (#10844)
7b34433 
* Revert "FIX: fixes regression where wizard dropdown couldn't update (#10838)"

This reverts commit e3b2fc6.

* Revert "DEV: replaces unecessary (action (mut .*)) by (mut .*) (#10822)"

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

@eviltrout eviltrout eviltrout approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jjaffeux @CvX @eviltrout