Merge lp://staging/~jelmer/storm/cast into lp://staging/storm

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jamu Kakar
Approved revision: 384
Merged at revision: 384
Proposed branch: lp://staging/~jelmer/storm/cast
Merge into: lp://staging/storm
Diff against target: 48 lines (+27/-0)
2 files modified
storm/expr.py (+20/-0)
tests/expr.py (+7/-0)
To merge this branch: bzr merge lp://staging/~jelmer/storm/cast
Reviewer Review Type Date Requested Status
Jamu Kakar (community) Approve
Gustavo Niemeyer Approve
Review via email: mp+42093@code.staging.launchpad.net

Commit message

Add storm.expr.Cast.

Description of the change

This branch adds storm.expr.Cast.

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Looks good. +1!

review: Approve
Revision history for this message
Jamu Kakar (jkakar) wrote :

[1]

Please add docstrings to the various bits you've added.

[2]

Maybe it's overkill, but I wonder if we should allow the second
parameter to optionally be a Python type? In such a case, we'd pick
the appropriate database type, depending on the backend.

+1 on this branch! If [2] seems interesting please file a bug about
it.

review: Approve
lp://staging/~jelmer/storm/cast updated
385. By Jelmer Vernooij

Add docstrings.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

I've added the docstring and filed a bug about supporting Python type objects. Is there anything else I need to do to help land this?

Revision history for this message
Jamu Kakar (jkakar) wrote :

Harassment was all that was needed, I've merged the changes. :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: