search

pgaskin / NickelMenu

The easiest way to launch scripts, change settings, and run actions on Kobo e-readers.
https://pgaskin.net/NickelMenu
MIT License
556 stars 28 forks source link

Rename chain to chain_success #33

Closed pgaskin closed 4 years ago

pgaskin commented 4 years ago

In hindsight, chain was a bad choice of name due to the changes in #20. One potential way to fix this while keeping backwards-compatibility would be to rename it to chain_success, but continue accepting chain. I would probably remove chain completely sometime before v1.0.0, though.

See https://github.com/geek1011/NickelMenu/pull/32#issuecomment-633654170.

pgaskin commented 4 years ago

I'd like some more opinions on this before I make a decision (about the option itself and about backwards-compatibility).

NiLuJe commented 4 years ago

I don't think many people are currently using this, and removing the action will very obviously generate a "config error" entry, which would make the deprecation fairly obvious and easy to fix for those few users caught unaware ;).

pgaskin commented 4 years ago

I think we currently have a few hundred users of NM (from the GH releases, we probably have at least that many from your OCP packages), but it's growing relatively quickly compared to my other projects (I think this may become more popular than the patches).

Of course, as you said, only a fraction will be using the chain functionality, as most general-purpose user-oriented features are part of single options. I'm mainly concerned with backwards compatibility due to the fact that NM will a larger percentage of ordinary (with less technical knowledge) users using it than things like kobopatch.

I'd add install-time telemetry to this (and kobopatch, too) if I was sure there wouldn't be backlash against it (privacy concerns and all that). It'd be useful to know the number of active users and which NM versions they are on (at least for firmware versions, I have the stats from kfwproxy, which seem to represent the version distribution well in general).

pgaskin commented 4 years ago

I think I'm going to rename it for v0.1.0, but emit a specific error for chain (which will eventually be removed).

pgaskin commented 4 years ago

While I'm at it, do you think I should also rename it to chain_ok and chain_failure to chain_err to make it shorter?

NiLuJe commented 4 years ago

I think I prefer success/failure (easier to grok, and readable as "chain on success"/"chain on failure") ;).