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

Add base webhook #5

Merged
merged 46 commits into from
Aug 29, 2016
Merged

Add base webhook #5

merged 46 commits into from
Aug 29, 2016

Conversation

jaumef
Copy link
Contributor

@jaumef jaumef commented Aug 25, 2016

References: #3
Added:

  • Base Webhook
  • Github Webhook
  • Gitlab Webhook (init)
  • Test data for:
  • Updated readme
  • Added Travis
  • Added python requirements
    • Added requirements-dev.txt with mamba
    • Added requirements.txt

Tests (using mamba):

  • Listener test
    • Instancer by payload
  • Webhook test
    • Basic Data
  • Gitlab webhook test
    • Basic Data
  • Github webhook test
    • Basic Data
    • status

Includes updated testing information
@XaviTorello
Copy link

XaviTorello commented Aug 25, 2016

@jaumef good work :)

Just some proposals to integrate on this PR:

  • Isolate the class webhook and the dispatcher (the executable, the one that will "proxify" the hooks execution) to provide an easy way to ensure and simplify heritance.
  • Provide an extended webhook object to clarify if it works as expected
    • i.e. _github extending webhook defining their own tests and actions to be done
  • There are no test data for github hook (at testing test_hooks.py)

What do you think about it?

@ecarreras
Copy link
Member

@jaumef we need the tests :trollface:

@XaviTorello
Copy link

XaviTorello commented Aug 26, 2016

Related to #3 and #4

  • Provide tests for each hook plz! ^^

@jaumef
Copy link
Contributor Author

jaumef commented Aug 29, 2016

@ecarreras @XaviTorello Beware, tests are incoming 👍

@jaumef jaumef removed the question label Aug 29, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b40fc33 on add_base_webhook into * on master*.

@ecarreras ecarreras merged commit 4764fd1 into master Aug 29, 2016
@ecarreras ecarreras deleted the add_base_webhook branch August 29, 2016 13:04
@XaviTorello
Copy link

Fixes #3 and closes #4

@jaumef 👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants