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

address #56 and change commit formatting to look more like the git hook #57

Merged
merged 1 commit into from
May 12, 2015

Conversation

Dav1dde
Copy link
Contributor

@Dav1dde Dav1dde commented Feb 12, 2014

This removes the omit phase options, it will always be the shown.
Adds what was requested in #56 (filtering for status), also changing the the BooleanFields to SelectMultipleField.

@TkTech
Copy link
Owner

TkTech commented Feb 12, 2014

Thanks for these changes, but I think it's a good idea to start requiring at least basic tests (using nosetest), especially since I have no Jenkins setup floating around to test. I'm thinking /tests, /tests/data/. /tests/data/ will have payload examples used in the tests. For the hooks, the tests are simple. Make a fake request, check the response.

@TkTech
Copy link
Owner

TkTech commented Feb 12, 2014

Thoughts?

@Dav1dde
Copy link
Contributor Author

Dav1dde commented Feb 12, 2014

I can definitely add tests to an existing infrastructure, since I never really worked with one of the bigger unittesting-frameworks apart from the std. unittest, I don't really know how that would look.

I imagine the tests (for Hooks) would require an easy way of influencing the parameters of handle_request, if that is in place, adding tests should be pretty easy and fast.

PS: I fixed the comment-issue and rebased the commits

@Dav1dde
Copy link
Contributor Author

Dav1dde commented Nov 5, 2014

So what is the current state? Tests?

TkTech added a commit that referenced this pull request May 12, 2015
address #56 and change commit formatting to look more like the git hook
@TkTech TkTech merged commit a55eeb7 into TkTech:master May 12, 2015
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