-
Notifications
You must be signed in to change notification settings - Fork 46
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
Priorities #168
Comments
Being one of the main people responsible for all that untested code making it into new features: I wholeheartedly agree, doing reviews w/o directly pushing to master is a thing we should do. 😃 For the features I'm a bit on the fence. Doesn't code review necessarily lead to people looking over newly added features before going live with them? |
Pull-requests for everyone sounds like a good idea, that gives the automated complainer a chance too. We haven't had new features in quite a while, maybe we stop having new features until X% test coverage is reached? |
@gedankenstuecke New features don't just have to be developed, though, they also have to be maintained, which is especially hard when untested. We saw the consequences of that when we upgraded to Rails 4. A lot of the untested code broke without us noticing before we deployed it. So, aiming at a certain test coverage might be a good idea. Definitely we shouldn't add or change any code without tests. Pull requests hopefully add some social pressure, too. ;) |
As we may have more new people contribute in the future: shall we codify this somewhere? |
Probably! :) |
I'll try to find the time for some editing of the README later tonight. :)
|
added a part on "no new features w/o tests", closes issue #168
Ok, that was not exactly "later tonight" but better late then never, right? 😂 |
🐝 |
🐝'n there, done that 😉 |
In the past we added every new feature that came to our minds, which has lead to a lot of messy, untested code. I would like to see this improve. For this, I think we should agree on at least some form of process regarding what code gets to be added. One thing we could do is proper code reviews, where no one pushes directly to master, but opens a pull request and at least on other developer looks over it and points out that there are tests missing... ;)
I'm not quite sure what to do about feature creep, though. I guess the same rules should apply: Every new feature is discussed in an issue first and we decide if it is really neccessary, if it can wait, or if we can live without it.
What do you think? Do you have other ideas?
The text was updated successfully, but these errors were encountered: