Closed jnphilipp closed 3 years ago
Needed to make some changes, to get it to work on my Ubuntu 20.04 with 3.36. Haven't tested it on other versions.
Needed to make some changes, to get it to work on my Ubuntu 20.04 with 3.36. Haven't tested it on other versions.
Thanks a lot. I think it is sufficient :)
@jnphilipp, @hedayat, thanks for your work on this, highly appreciated.
Allow me to make a critical remark nonetheless. I wouldn't have acked this if I had looked at it before it was merged. Being late to the party, I can't seriously complain, but please mind the following for future PRs:
extension/prefs.js
. Apparently you added visible: true
properties to various widgets. That was probably a correct change, but changes like this should go into a separate commit with appropriate message, rather than being hidden in a merge commit. Moreover, in a PR branch you should rebase the PR if upstream changes, rather than merging. It makes reading the history much easier later on.OK.
To be honest, I didn't care about the merge instead of rebase thing, and while I myself usually do that, I'm not sure if it is good thing to require it.
Having separate commits is OK, I am just not knowledgeable enough about this JS/shell thing to decide about the boundries. :P
About the header, well, I'm really sorry, I'm not sure if I've even read it carefully back then :D Makeing is just too weird to even need a spell checker :P
Dear @jnphilipp , would you please resolve the conflict and let me know if this extension is compatible with older versions?