urob / zmk-helpers

Convenience macros simplifying ZMK's keymap configuration
MIT License
235 stars 79 forks source link

Remove deprecated `label` property #33

Closed bryanforbes closed 5 months ago

urob commented 9 months ago

Thanks for the PR. I am a bit hesitant to merge it just yet because I don't have the label-refactor commit in my main zmk branch, which I think many folks use in their GHA workflows. So merging this now would risk breaking their workflow if they were to pull this repo.

The deeper reason why I haven't updated my zmk branch yet is that I am waiting for the upstream mouse work to be completed so that I can rebase without downgrading the mouse functionality (I really should just rebase anyhow and replace the partial mouse work in upstream with the older PR version -- but I haven't gotten to it yet, and was hoping that the wait wouldn't be too much longer anyhow).

So not fully sure what's the best time to merge this. What do you think? Maybe we should merge this in a separate branch for now?

bryanforbes commented 9 months ago

The deeper reason why I haven't updated my zmk branch yet is that I am waiting for the upstream mouse work to be completed so that I can rebase without downgrading the mouse functionality (I really should just rebase anyhow and replace the partial mouse work in upstream with the older PR version -- but I haven't gotten to it yet, and was hoping that the wait wouldn't be too much longer anyhow).

There’s a backport of the remaining mouse functionality at https://github.com/caksoylar/zmk/tree/feat/mouse-keys-3.2 if you want to rebase on main. I can understand the hesitancy, though.

So not fully sure what's the best time to merge this. What do you think? Maybe we should merge this in a separate branch for now?

Since the current code doesn’t break the build on ZMK main (it just emits deprecation warnings), there’s no need to merge this now. Merging it to a separate branch would be fine, but leaving this unmerged until you have the relevant commit in your ZMK fork seems acceptable as well.

urob commented 5 months ago

Oops, this fell out of sight. Just merged it, it's definitely overdue now. Thanks again