Skip to content
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.

Sign up for GitHub

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 #449] Use the new GitHub's PR review feature #457

Merged
merged 1 commit into from
Jul 6, 2017
Merged

[Fix #449] Use the new GitHub's PR review feature #457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ found [here](https://github.com/inaka/elvis/wiki/Default-Rules-in-Elvis-Webhook)
#### Running the webhook on your servers

Since GitHub's API needs a valid user and password to allow the creation of
comments on PRs, the parameters `github_user` and `github_password` need to be
added to `elvis`'s [configuration](#configuration).
reviews on PRs, the parameters `github_user` and `github_password` need to be
added to `elvis`'s [configuration](#configuration) and also the credentials used
must be from an admin of the repo or someone with permissions for requesting changes
on PRs.

The `webhook/1` function takes a map containing the keys `headers` and `body`,
whose values should be the map of headers and the body from the GitHub's event
Expand Down
5 changes: 4 additions & 1 deletion config/test.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
}
]
},
{output_format, plain}
{output_format, plain},
%% Basic Auth
{github_user, ""},
{github_password, ""}
]
}
].
2 changes: 1 addition & 1 deletion rebar.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@

{deps, [ {elvis, "0.3.8", {pkg, elvis_core}}
, {getopt, "0.8.2"}
, {egithub, "0.3.2"}
, {egithub, "0.5.1"}
, {katana_code, "0.1.0"}
]}.

Expand Down
6 changes: 3 additions & 3 deletions rebar.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{"1.1.0",
[{<<"aleppo">>,{pkg,<<"inaka_aleppo">>,<<"1.0.0">>},1},
{<<"certifi">>,{pkg,<<"certifi">>,<<"0.7.0">>},2},
{<<"egithub">>,{pkg,<<"egithub">>,<<"0.3.2">>},0},
{<<"egithub">>,{pkg,<<"egithub">>,<<"0.5.1">>},0},
{<<"elvis">>,{pkg,<<"elvis_core">>,<<"0.3.8">>},0},
{<<"getopt">>,{pkg,<<"getopt">>,<<"0.8.2">>},0},
{<<"goldrush">>,{pkg,<<"goldrush">>,<<"0.1.9">>},2},
{<<"goldrush">>,{pkg,<<"goldrush">>,<<"0.1.9">>},1},
{<<"hackney">>,{pkg,<<"hackney">>,<<"1.6.5">>},1},
{<<"idna">>,{pkg,<<"idna">>,<<"1.2.0">>},2},
{<<"jiffy">>,{pkg,<<"jiffy">>,<<"0.14.11">>},1},
Expand All @@ -18,7 +18,7 @@
{pkg_hash,[
{<<"aleppo">>, <<"8DB14CF16BB8C263C14FF4C3F69F64D7C849D40888944F4204D2CA74F1114CEB">>},
{<<"certifi">>, <<"861A57F3808F7EB0C2D1802AFEAAE0FA5DE813B0DF0979153CBAFCD853ABABAF">>},
{<<"egithub">>, <<"104088B9BBEF7B637A6A2F8DAF59A6201B5FA18EC86F10B4739CDEFF7D0BD492">>},
{<<"egithub">>, <<"C8BDB55F4D9EF610535D62CC96A43D974BE8D5924C5507218A25B1DF608788ED">>},
{<<"elvis">>, <<"9FE9DBFF15150DB41110D86BD87F29A7269A5CC3C0984EF47C1D208FBBD24222">>},
{<<"getopt">>, <<"B17556DB683000BA50370B16C0619DF1337E7AF7ECBF7D64FBF8D1D6BCE3109B">>},
{<<"goldrush">>, <<"F06E5D5F1277DA5C413E84D5A2924174182FB108DABB39D5EC548B27424CD106">>},
Expand Down
64 changes: 42 additions & 22 deletions src/elvis_webhook.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@ event(Cred, Request) -> egithub_webhook:event(?MODULE, Cred, Request).
-spec handle_pull_request(
egithub:credentials(), egithub_webhook:req_data(),
[egithub_webhook:file()]) ->
{ok, [egithub_webhook:message()]} | {error, term()}.
{ok, egithub_webhook:review()} | {error, term()}.
handle_pull_request(Cred, Data, GithubFiles) ->
#{<<"repository">> := Repository} = Data,
BranchName = maps_get([ <<"pull_request">>,
<<"head">>,
<<"ref">>],
Data, <<"master">>),
BranchName =
maps_get([<<"pull_request">>, <<"head">>, <<"ref">>], Data, <<"master">>),
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure <<"master">> should be hardcoded here?
There are PRs that don't go against master.

Copy link
Member

Choose a reason for hiding this comment

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

my bad, I didn't understand the code

Repo = binary_to_list(maps:get(<<"full_name">>, Repository)),
Branch = binary_to_list(BranchName),
Config = repo_config(Cred, Repo, Branch, elvis_config:default()),
Expand All @@ -39,10 +37,17 @@ handle_pull_request(Cred, Data, GithubFiles) ->

FileInfoFun = fun (File) -> file_info(Cred, Repo, File) end,
Config2 = elvis_config:apply_to_files(FileInfoFun, Config1),
CommitId =
maps_get([<<"pull_request">>, <<"head">>, <<"sha">>], Data, <<>>),

case elvis_core:rock(Config2) of
{fail, Results} -> {ok, messages_from_results(Results)};
ok -> {ok, []}
{fail, Results} ->
{ok, review_from_results(Results, CommitId)};
ok ->
{ok, #{commit_id => CommitId,
body => <<":+1:">>,
event => <<"APPROVE">>,
comments => []}}
end.

-spec handle_error( {error, term()}
Expand All @@ -54,6 +59,23 @@ handle_error({error, _}, _, _) -> {ok, [], []}.
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%% Helper functions

-spec review_from_results([elvis_result:file()], string()) ->
egithub_webhook:review().
review_from_results(Results, CommitId) ->
RC = review_comments_from_results(Results),
%% Comments with `position = 0' cannot be added as a review comment, so they
%% are added to the review's body.
Fun = fun(#{position := 0, body := B}, {BodyAcc, CommentsAcc}) ->
{<<BodyAcc/binary, "\n- ", B/binary>>, CommentsAcc};
(Comment, {BodyAcc, CommentsAcc}) ->
{BodyAcc, [Comment | CommentsAcc]}
end,
{Body, ReviewComments} = lists:foldl(Fun, {<<>>, []}, RC),
#{commit_id => CommitId,
body => Body,
event => <<"REQUEST_CHANGES">>,
comments => ReviewComments}.

-spec github_credentials() -> egithub:credentials().
github_credentials() ->
User = application:get_env(elvis, github_user, ""),
Expand Down Expand Up @@ -88,47 +110,45 @@ commit_id_from_raw_url(Url, Filename) ->
{match, [_, {Pos, Len} | _]} = re:run(UrlString, Regex),
string:substr(UrlString, Pos + 1, Len).

messages_from_results(Results) ->
review_comments_from_results(Results) ->
lists:flatmap(
fun(Result) ->
messages_from_result(Result)
review_comments_from_result(Result)
end, Results).

messages_from_result(Result) ->
review_comments_from_result(Result) ->
File = elvis_result:get_file(Result),
Rules = elvis_result:get_rules(Result),
lists:flatmap(
fun(Rule) ->
messages_from_result(Rule, File)
review_comments_from_result(Rule, File)
end, Rules).

messages_from_result(Rule, File) ->
review_comments_from_result(Rule, File) ->
Items = elvis_result:get_items(Rule),
lists:flatmap(
fun(Item) ->
messages_from_item(Item, File)
review_comments_from_item(Item, File)
end, Items).

messages_from_item(Item, File) ->
#{path := Path,
commit_id := CommitId,
patch := Patch} = File,
review_comments_from_item(Item, File) ->
#{path := Path, patch := Patch} = File,
Message = elvis_result:get_message(Item),
Info = elvis_result:get_info(Item),
Line = elvis_result:get_line_num(Item),
Text = list_to_binary(io_lib:format(Message, Info)),

case Line of
0 ->
[#{text => Text,
position => Line}];
[#{path => Path,
position => Line,
body => Text}];
_ ->
case elvis_git:relative_position(Patch, Line) of
{ok, Position} ->
[ #{commit_id => CommitId,
path => Path,
[ #{path => Path,
position => Position,
text => Text
body => Text
}
];
not_found ->
Expand Down