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.

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

make egithub_webhook:request() type opaque #131

Merged
merged 1 commit into from
Nov 1, 2016

Conversation

mfelsche
Copy link

and move creation and extraction into its own module.

this solves #117

though i actually didn't see any place in erlang-githubs codebase where a requests body was accessed multiple times.

%%%
%%% @end
%%% Created : 28. Oct 2016 11:11 AM
%%%-------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove or fix these comments (from line 1 to 8). I'd let only de @doc section, so you can add a brief description about the module.

%%%
%%% @end
%%% Created : 28. Oct 2016 11:40 AM
%%%-------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove or fix these comments (from line 1 to 8). I'd let only de @doc section, so you can add a brief description about the module.

@@ -0,0 +1,40 @@
%%%-------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Please translate this test from EUnit to Common Test.

@cabol
Copy link
Contributor

cabol commented Oct 31, 2016

@mfelsche please update your branch, I think there aren't conflicts, but better be sure!

@mfelsche mfelsche force-pushed the opaque_webhook_request branch from 5e04292 to 337d04a Compare October 31, 2016 20:11
and move creation and extraction into its own module
@mfelsche mfelsche force-pushed the opaque_webhook_request branch from 337d04a to 29800b9 Compare October 31, 2016 20:13
@mfelsche
Copy link
Author

i addressed all the comments.
sorry for the stupid docstrings.
moved the testsuite to common test.

@cabol
Copy link
Contributor

cabol commented Nov 1, 2016

@elbrujohalcon from my side, I see everything OK.

@cabol cabol merged commit 4d82b72 into inaka:master Nov 1, 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