Add support for BYSETPOS #1
Conversation
A few small updates to Nicolas Marlier's BYSETPOS support added in PR ice-cube-ruby#349
| require "date" | ||
| require "time" | ||
| require "active_support" | ||
| require "active_support/core_ext" |
There was a problem hiding this comment.
It looks like upstream won't accept this code if it uses ActiveSupport, since they want to keep it pure ruby.
ice-cube-ruby#449 (comment)
ice-cube-ruby#449 (comment)
ice-cube-ruby#449 (comment)
So if we did want to push this upstream we might need to replace them with pure ruby, but since that PR is still getting updates maybe we won't need to. So we can keep ActiveSupport for now and wait for upstream to fix & merge their version.
There was a problem hiding this comment.
Achieve functionality first then optimise later.
That's a good rule. The only thing is that tech-debt can hide under the radar. But, still, you probably want the functionality first. So,
Achieve functionality first then remove tech-debt and optimise later.
It would be nice to push this upstream, but we don't really need to. So, let's deal with that later too. 👍
|
Pull request #2 |
This code was written and submitted as a pull request upstream in 2019 but hasn't had any attention since.
I pulled those changes, applied a couple of other fixes from testing and until upstream merges this change, we'll need to maintain it ourselves.
Followup: update the app to pull from this repo instead of rubygems.