servo / highfive

Github hooks to provide an encouraging atmosphere for new contributors
Mozilla Public License 2.0
254 stars 58 forks source link

Support file path exclusions in watcher handler. Issue #76 #79

Closed mylainos closed 8 years ago

mylainos commented 8 years ago

I have used this https://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops. I don't understand how the test work, how should I add one ?

highfive commented 8 years ago

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon.

highfive commented 8 years ago

Heads up! This PR modifies the following files:

jdm commented 8 years ago

Does https://github.com/servo/highfive#testing clear up the test format? The JSON files provide an initial state of the PR (such as the current assignee and labels), then process the given payload, and assert that the PR's final state matches the expected one (ie. number of comments added, assignee, labels, etc.).

mylainos commented 8 years ago

I have seen this. The tests in /handlers/watchers/tests/new_pr.json are specifics, should I do something like that:

"diff": "diff --git a/watched/dir/file.rs /dev/null"

I don't understand what this mean and why he expect a number of comments. EDIT: I have understand now.

jdm commented 8 years ago

Ah. That's a very simple mock of a PR diff that contains changes to a file that is watched (in this case, the /dev/null means that it's being deleted). The expected number of comments comes from the handler publishing a comment announcing that the PR changes a file that is being watched.

mylainos commented 8 years ago

Should I squash ?

jdm commented 8 years ago

No need, github lets me do that. Thanks for the PR!

jdm commented 8 years ago

cc @jgraham