Closed Tensho closed 10 years ago
I'm all for removing App.user_cache
, however I'm going to politely request that the BW maintainers do NOT merge this PR since it's a breaking change. If I upgrade my version of BW and push it to my users they'll lose ALL their saved settings since I'm using App::Persistence
.
Yeah, the App::Persistence change would not be good unless this is explicitly included in a major point release that details breaking API changes. I'm also not super clear on why it's necessary to change this.
Agreed with @markrickert that it needs to be backwards compatible.
May I suggest some vestigial code checks like @clayallsopp did in Formotion? https://github.com/clayallsopp/formotion/blob/ecd56bdac1867c8331e44b82fa8dec33bcf8e7bf/lib/formotion/form/form.rb#L327
I'm pretty in-line with @markrickert and @gertig on this. This solves a very minor inconvenience by introducing a critical backwards compatibility problem. I'm not sure if I'd ever want to change how the App::Persistence
key works at this point, even we did it in accordance with semver.
I am for deprecating user_cache
with a message in the next release of BW and removing it in BW 2.0
OK, I agree with everything.
Related to #345