Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Define Box.get #1523

Merged
merged 3 commits into from Apr 7, 2014

Conversation

Projects
None yet
5 participants
Owner

Shadowfiend commented Apr 6, 2014

Folks frequently use .get on Box, triggering the implicit Box->Option conversion
and thus both making it easier to open the box (bad) and making that process slower
(baddd).

Let's define a get method on Box that is declared to return an internal class and
throws an Exception. This will fail compiler checks and also not work at runtime. For
2.6, we can define a get method that works like open_! and immediately deprecate it.

Owner

fmpwizard commented Apr 1, 2014

👍 the idea and for those who really want to call .get they can to toOption.get

Contributor

nafg commented Apr 2, 2014

You can deprecate it immediately, why would you want to make it work like
open_!?

That is, in Lift-paranoia land.

On Tue, Apr 1, 2014 at 2:15 PM, Diego Medina notifications@github.comwrote:

[image: 👍] the idea and for those who really want to call .get they
can to toOption.get

Reply to this email directly or view it on GitHubhttps://github.com/lift/framework/issues/1523#issuecomment-39239306
.

Member

tuhlmann commented Apr 2, 2014

👍 .get is used in numerous other contexts in Lift and I can see that it
is just mistakenly used without knowledge of the dangers. Good to prevent
that.

2014-04-02 3:32 GMT+02:00 nafg notifications@github.com:

You can deprecate it immediately, why would you want to make it work like
open_!?

That is, in Lift-paranoia land.

On Tue, Apr 1, 2014 at 2:15 PM, Diego Medina <notifications@github.com

wrote:

[image: 👍] the idea and for those who really want to call .get they
can to toOption.get

Reply to this email directly or view it on GitHub<
https://github.com/lift/framework/issues/1523#issuecomment-39239306>

.

Reply to this email directly or view it on GitHubhttps://github.com/lift/framework/issues/1523#issuecomment-39280519
.

AGYNAMIX(R). Passionate Software.
Inh. Torsten Uhlmann | Buchenweg 5 | 09380 Thalheim
Phone: +49 3721 273445
Fax: +49 3721 273446
Mobile: +49 151 12412427
Web: http://www.agynamix.de
Author of "Lift Web Applications
How-Tohttp://www.packtpub.com/lift-web-applications/book
"

Owner

Shadowfiend commented Apr 2, 2014

Deprecation will be immediate. Changing of types will not happen in 2.6. Basic release planning in action in Lift-paranoia land ;)

Contributor

nafg commented Apr 3, 2014

Ah, I misread before, the "2.6" didn't register properly and I thought it
was something happening later rather than earlier. Now it makes sense. :)

On Wed, Apr 2, 2014 at 2:48 PM, Antonio Salazar Cardozo <
notifications@github.com> wrote:

Deprecation will be immediate. Changing of types will not happen in 2.6.
Basic release planning in action in Lift-paranoia land ;)

Reply to this email directly or view it on GitHubhttps://github.com/lift/framework/issues/1523#issuecomment-39368474
.

Shadowfiend added some commits Apr 6, 2014

@Shadowfiend Shadowfiend Fix Box->Option implicit conversion spec.
We were testing Empty to ensure isDefined returned false, but
isDefined hasn’t triggered an implicit conversion to Option in a
while. We now test the implicit conversion by verifying that
orElse exists and behaves as expected on Options.
b89d124
@Shadowfiend Shadowfiend Fix a reference to open_! in the Box specs. 38866be
@Shadowfiend Shadowfiend Define a get method on Box and immediately deprecate it.
We do this so that get no longer triggers an implicit conversion
to Option, and triggers a deprecation warning. In the future,
get will remain defined but will throw an exception and be
defined as returning an internal vestigial type to break
compilation.

Those who still want to use .get can call .toOption.get.
0bdee65
Owner

Shadowfiend commented Apr 6, 2014

Added some code to implement the deprecated version of this.

Owner

Shadowfiend commented Apr 6, 2014

I also pushed some code to implement the Lift 3 side of this, in branch lift-3-box-get-out, but I'll hold off on the PR until this gets merged into 2.6 and I can properly layer the change commits on top of the result.

Owner

farmdawgnation commented Apr 7, 2014

Code looks good. Going to give this guy a quick text then merge.

@farmdawgnation farmdawgnation added this to the 2.6-M3 milestone Apr 7, 2014

Owner

farmdawgnation commented Apr 7, 2014

it fits

👍

@farmdawgnation farmdawgnation added a commit that referenced this pull request Apr 7, 2014

@farmdawgnation farmdawgnation Merge pull request #1523 from lift/1523-gettin-boxed-in
Define a Box.get that is deprecated to discourage people from using it since it's not the "correct" way to open a box.
68363a0

@farmdawgnation farmdawgnation merged commit 68363a0 into master Apr 7, 2014

@farmdawgnation farmdawgnation deleted the 1523-gettin-boxed-in branch Apr 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment