New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 2004: DictionaryList.get doesn't work with class/interface #2005

Merged
merged 3 commits into from Dec 21, 2017

Conversation

Projects
None yet
3 participants
@quickfur
Contributor

quickfur commented Dec 14, 2017

Turns out that the fix for issue #2004 is trivial: the default argument just needs to be specified with the inout qualifier.

@dlang-bot dlang-bot added the Bug Fix label Dec 14, 2017

@s-ludwig

This comment has been minimized.

Member

s-ludwig commented Dec 14, 2017

Thanks for the PR! Looks like this should indeed work on all DMD versions. Can you do a quick rebase on top of the current master, though? Today something on the Travis-CI image changed, breaking the build, and the workaround has just been merged

@quickfur

This comment has been minimized.

Contributor

quickfur commented Dec 14, 2017

Rebased.

@s-ludwig s-ludwig added the auto-merge label Dec 14, 2017

@@ -400,3 +400,15 @@ unittest {
assert(text(l) == `["foo": 42, "bar": 43]`, text(l));
assert(l.toString() == `["foo": 42, "bar": 43]`, l.toString());
}
// Issue 2004
@safe unittest {

This comment has been minimized.

@s-ludwig

s-ludwig Dec 15, 2017

Member

I think the failures are coming from Variant not being annotated @trusted. Since this is just about the inout issue, I'll just quickly remove the @safe.

@dlang-bot dlang-bot removed the auto-merge label Dec 15, 2017

@s-ludwig s-ludwig added the auto-merge label Dec 15, 2017

@quickfur

This comment has been minimized.

Contributor

quickfur commented Dec 20, 2017

Whoa, this is still sitting here? What gives with the AppVeyor problem? Anything I can do on my end?

@dlang-bot dlang-bot removed the auto-merge label Dec 20, 2017

@quickfur

This comment has been minimized.

Contributor

quickfur commented Dec 20, 2017

Saw some commits related to working around AppVeyor issues; rebased on master to see if that helps.

@s-ludwig s-ludwig added the auto-merge label Dec 21, 2017

@dlang-bot dlang-bot merged commit a763bc0 into vibe-d:master Dec 21, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@quickfur quickfur deleted the quickfur:dictionary_get_class branch Dec 21, 2017

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