search

mirage / ocaml-github

GitHub APIv3 OCaml bindings
ISC License
100 stars 61 forks source link

Add support for organisation hooks #181

Closed samoht closed 7 years ago

samoht commented 7 years ago

I tried to not break the API by adding optional arguments. Not sure if it is the best choice.

dsheets commented 7 years ago

Along with #180, this will take us to 3.0. In the process, we might as well split the hook module into nested modules under the modules that describe the types the events concern (i.e. Hook splits into Repo.Hook and Organization.Hook).

This is significantly more work than this PR alone but I think it's probably a better design with fewer run-time errors and a clearer organization. What do you think?

samoht commented 7 years ago

I agree, I am not very happy with the optional arguments in that PR. I'll rework that.

dsheets commented 7 years ago

I think deprecating the Hook module and adding object-specific hook modules makes sense and does not require interface changes or nasty types. What do you think?