Skip to content

Conversation

wilzbach
Copy link
Contributor

With the improvement ideas from #15

if (runAsync)
runTask(fun, args);
else
return fun(args);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the async processing isn't really safe and might loose jobs, maybe we never want to do things asynchronously?

I don't know how "safe" Vibe.d's runTask is, so leaving this decision to you.

"/github/repos/dlang/phobos/issues/4921/comments",
"/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc",
"/github/repos/dlang/phobos/issues/comments/262784442",
(scope HTTPServerRequest req, scope HTTPServerResponse res){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately just (scope req, scope res) doesn't work, because D isn't smart enough yet to figure the types out automatically, but imho this is nice enough for the beginning?

Copy link
Member

@MartinNowak MartinNowak Dec 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine and also comes with clearer error messages.

Technically it's possible to pass a polymorphic lambda to a template and check whether it's instantiatable though.
The error messages are not so good, b/c you can't know which type was meant.

void foo(Args...)()
{
    static if (__traits(compiles, Args[0](12)))
    {
        pragma(msg, "int");
    }
    else static if (__traits(compiles, Args[0]("string")))
    {
        pragma(msg, "string");
    }
}

void test()
{
    foo!(a => 2 * a);
    foo!(a => a ~ "foo");
}

@@ -0,0 +1,82 @@
import app;
import utils;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh the GitHub file detection is still broken:

github-linguist/linguist#3145

if (expectation.reqHandler !is null)
return expectation.reqHandler(req, res);

string filePath = buildPath(payloadDir, req.requestURL[1 .. $].replace("/", "_"));
Copy link
Contributor Author

@wilzbach wilzbach Dec 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved all payloads in one folder and use this / -> _ replacement rule, s.t. one can have a good overview of all available payloads.
It's still simple to fetch new requests, e.g.

api=repos/dlang/dmd/pulls/6328/commits && curl https://github.com/api/${api} > data/payloads/github_$(echo $api | tr '/' '_')

However if you prefer to have an exact 1:1 matching with folders, I wouldn't mind either.

edit: Note that to simplify testing I added the option to provide a jsonHandler and thus modify the json output - or in other words. We will have to do file reading manually anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine for now.

@MartinNowak MartinNowak merged commit 9c7df03 into dlang:master Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants