search

embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
329 stars 137 forks source link

Add support for `{{unless}}` to the `macroCondition` macro #1858

Closed Windvis closed 1 month ago

Windvis commented 1 month ago

I noticed the macroCondition macro could only be used with the {{if}} helper / keyword. This change makes it possible to use {{unless}} as well.

(Not a very DRY change, but it seemed better to keep things simple and duplicate the if version 😄.)

Windvis commented 1 month ago

@ef4 is there anything that needs to happen before this can be merged or did this simply fall through the cracks 😄?

mansona commented 1 month ago

@Windvis one thing that might be slowing this down is that you're targeting main with this change 🤔 are you intending to make use of this soon? main isn't going to be released any time soon and there are a lot of breaking changes, if you want to use this soon then I would target stable

Windvis commented 1 month ago

@mansona No, there is no rush since it's easy to work around the issue.

If I target stable, how does the change get back into main? Does it get cherry picked, or do I need to open a second PR that targets main?

mansona commented 1 month ago

@Windvis once things are released in stable we forward-port them to main. It tends to be a lot easier to do it this way, also since we're keeping things up to date often enough 👍

Essentially you would not need to do anything extra, it would be on us :)

ef4 commented 1 month ago

Let's try to retarget this to stable, @mansona has volunteered, it looks like it will apply cleanly.