Re: Commented-out tests?
On 29 August 2014 20:55, Nate Finch nate.fi...@canonical.com wrote: Git blame says ask Roger ;) Sorry about that. I suspect I commented them out when trying to get some changeset to pass tests, but forgot to uncomment them and they were missed in review. I agree with not leaving commented out tests around, but I would prefer if the tests were uncommented and made to pass (fixing whatever is necessary to make them pass while thinking through to what logic they were supposed to be testing in the first place). For the record, the commented out tests were introduced here: https://codereview.appspot.com/13355044/diff/53001/environs/config/config_test.go As a further comment, in the description of that CL, it says Still not done: providers still take defaults from the environment. I suspect that still awaits completion. cheers, rog. PS this is a good example of why it's good to avoid the use of /* */ comments, even temporarily. If I'd used //, the commented out code would have been much more obvious. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Fwd: Commented-out tests?
On 29 August 2014 20:55, Nate Finch nate.fi...@canonical.com wrote: Git blame says ask Roger ;) Sorry about that. I suspect I commented them out when trying to get some changeset to pass tests, but forgot to uncomment them and they were missed in review. I agree with not leaving commented out tests around, but I would prefer if the tests were uncommented and made to pass (fixing whatever is necessary to make them pass while thinking through to what logic they were supposed to be testing in the first place). For the record, the commented out tests were introduced here: https://codereview.appspot.com/13355044/diff/53001/environs/config/config_test.go As a further comment, in the description of that CL, it says Still not done: providers still take defaults from the environment. I suspect that still awaits completion. cheers, rog. PS this is a good example of why it's good to avoid the use of /* */ comments, even temporarily. If I'd used //, the commented out code would have been much more obvious. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Commented-out tests?
Hey all, I ran into some commented out tests while making a change: https://github.com/juju/juju/pull/630/files#r16874739 I deleted them since keeping things around that we might need later is the job of source control, not comments ;) Ian just wanted me to check just to make sure this was OK. If not, please chime in on the PR ASAP. Thanks all! - Katherine -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Commented-out tests?
Git blame says ask Roger ;) 100% agree... don't leave in commented out code. If you feel you must, because you expect it to get uncommented very soon, then leave a very clear comment about why it was commented out and when it should get uncommented. But mostly just don't. On Fri, Aug 29, 2014 at 3:28 PM, Katherine Cox-Buday katherine.cox-bu...@canonical.com wrote: Hey all, I ran into some commented out tests while making a change: https://github.com/juju/juju/pull/630/files#r16874739 I deleted them since keeping things around that we might need later is the job of source control, not comments ;) Ian just wanted me to check just to make sure this was OK. If not, please chime in on the PR ASAP. Thanks all! - Katherine -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Commented-out tests?
On 30/08/14 05:55, Nate Finch wrote: Git blame says ask Roger ;) 100% agree... don't leave in commented out code. If you feel you must, because you expect it to get uncommented very soon, then leave a very clear comment about why it was commented out and when it should get uncommented. But mostly just don't. Agree. I was curious as to why they were commented out - in the absence of comments, I didn't know if people were planning on adding them back Real Soon or if the code truly was obsolete. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Commented-out tests?
On Fri, Aug 29, 2014 at 4:28 PM, Katherine Cox-Buday katherine.cox-bu...@canonical.com wrote: Hey all, I ran into some commented out tests while making a change: https://github.com/juju/juju/pull/630/files#r16874739 I deleted them since keeping things around that we might need later is the job of source control, not comments ;) If it was a relevant test, removing them generally means they're never coming back. The best course of action might be to Skip it or to use ExpectFailure, providing an appropriate reason string. This makes it visible that there are tests not being run or failing, while still making sure they at least build. Of course, if they're indeed never coming back, then just removing them for good is more honest. gustavo @ http://niemeyer.net -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
