Monday, 2014-07-28

*** mlncn has joined #npoacct00:56
*** bkuhn has joined #npoacct04:28
*** bkuhn has quit IRC06:18
*** mlinksva has quit IRC10:50
*** tgnit has joined #npoacct16:17
*** tgnitidle has quit IRC16:20
*** bkuhn has joined #npoacct17:38
bkuhntgnit: greetings.17:40
tgnitbkuhn:  greetings!17:41
bkuhntgnit: have you thought more about how you want to proceed?17:43
tgniti am writing unit test for balance. cc  :)17:45
bkuhnOk, that makes sense.17:46
bkuhnI'm sorry that the feature was so complicated.17:46
bkuhntgnit: One thing I would like you to do before summers' end as well then...17:46
tgnitbkuhn :  i am done with sorting balance object by commodity  will wait for  decision taken on mailing list17:46
bkuhn.... and I know writing "papers" is tough  ...17:46
bkuhn... is write a one-page report on what you tried.17:46
tgnitsure!17:46
bkuhnthanks.17:47
bkuhntgnit: don't worry; this feature turned out to be MUCH more complicated than we expected.17:47
bkuhnI talked some with johnw about it.17:47
tgnitwhat he said?17:47
bkuhnWell, mainly that it's not clear what this feature should do.17:47
bkuhnThat we're all confused about the implications.17:47
bkuhnWhich I think both tbm and I are.17:48
bkuhnWe keep finding new issues that he and I didn't anticipate.17:48
bkuhnjohnw is reluctant to add any feature upstream that we don't understand all the use-case and implications of.17:48
bkuhnThis happens sometimes in Free Software.17:48
bkuhnYou did a good job, sometimes, your patches don't get accepted upstream.17:48
bkuhnBut, you're right to do it:17:48
bkuhnPrepare the "sorting version" as a final proposal.17:49
bkuhnGo ahead and *do* submit a merge request, "for the record".17:49
bkuhnBut understand that johnw will probably have to reject it.17:49
bkuhnThat's not failure, many many changes get rejected in Free Software. :)17:49
tgnitbkuhn:  i understand!17:49
bkuhnI mean, to be honest, it *is* failure, but it's not a failure of your GSoC time: you've learned that sometimes, you write your best patch and upstream won't take it. ;)17:49
bkuhnThere has been a lot written that we learn well from failure.17:50
bkuhnI think you did your best, and I am happy with your work.17:50
bkuhnI can imagine I'd have done the same approach myself and get it rejected.17:50
bkuhnIf you had more time for GSoC, we'd have you try again.17:50
bkuhnSo, tgnit, thus the plan is:17:50
tgnitbkuhn:  i think johnw is right by doing it,  you can't merge a thing you can't explain17:50
bkuhn:)17:50
bkuhnso the plan is:17:50
bkuhn0. You'll finish the patch, upstream it, and johnw will likely reject.17:50
bkuhn1. You'll write a one-page paper about your experience preparing the patch, what you learned, etc.17:51
bkuhn2. You'll write at least some tests in the final weeks that are good enough to get merged upsteram.17:51
bkuhn*upstream17:51
bkuhnIf you do 0-2 successfully, I will pass you for GSoC.17:51
tgnitbkuhn:  got you17:52
bkuhnFor (2), I suggest upstreaming the tests one-by-one, or in small groups of 1-2 tests.17:52
tgnitthanks for making it clear17:52
bkuhnSure!  I think you've done a reasonable job.  Neither tbm nor I understood how complicated this feature would be.  I believe you did your best.17:52
bkuhnOk, let me know if you need any help or questions on 0-2 above.17:53
tgnitbkuhn:  thanks !17:53
bkuhnLet's check in again on, say, Thursday, to see where you are on that?17:53
tgnitsure!17:53
tgnitcan you check code_coverage branch  for once?17:54
tgniton https://github.com/tripun/ledger17:55
bkuhnsure, what specifically do you want me to look at?17:55
* bkuhn does a pull17:55
tgnitjust the latest commits.17:55
tgnitunit test on balance. cc17:55
tgniti have tried they are working fine17:56
bkuhnOh, I see johnw accepted your change for acprep!  Good work! :)17:57
bkuhnI see you're adding unit tests to the C++ unit test suit.17:58
bkuhn*suite17:58
bkuhnthat's fine.17:58
bkuhnI was thinking more about adding tests to the regression tests too.17:58
bkuhnI guess there is no code-coverage run of the regression tests?17:59
bkuhnI.e., is there a report that can be generated that show how much the regression tests cover the code?17:59
bkuhnOr does the test report actually show coverage of the entire unit test + regression tests?17:59
bkuhnI actually don't know (I should, I know :)17:59
bkuhnI've added lots of tests to Ledger, but I never ran the coverage report myself. :)17:59
tgnitthere is code coverage but i have not generated new report18:00
tgnitbkuhn:  i started with balance. cc as the unit that was empty18:00
bkuhnYeah, that's smart.18:03
bkuhnSo, I think you should do a mix, though, of regression and unit tests.18:04
bkuhnto boost coverage.18:04
tgnitok18:04
tgniti need to check as to what tests are missing in regression though18:05
bkuhnYeah, I am still unclear; is there a report that does a gcov report for the regression tests, and/or "the regression and unit tests together"?18:05
tgnitits a total report18:10
bkuhnah, ok.18:10
bkuhnthat's good.18:10
bkuhnSo, I'd encourage you to add a mix of unit tests and regression tests.18:10
bkuhnI'd suggest as above to offer merges to johnw every 2-3 days.18:10
tgnitneed to check how to get coverage only for specific tests18:10
tgnitok18:11
bkuhnYeah, I'm not *terribly* worried if we get coverage reports for specific tests, if the coverage report is for the whole suite already.18:11
bkuhnJust write tests of all types.18:11
tgnitsure!18:12
bkuhntgnit: for the moment, maybe write a few more unit tests /test/unit/t_balance.cc and submit that file as a merge request?18:13
bkuhnThen, while you're waiting for johnw to review that, work on some new regression tests.18:14
bkuhnThat way if johnw has feedback on your unit tests that require a change in how you're doing them, you won't have too much extra work to fix.18:14
tgnitright18:14
bkuhnthen, you hopefully will have regression tests to merge, and you can keep switching back and forthn.18:14
tgnitthat is right way to do it.18:15
tgnitbkuhn:  the sorting and rounding stuff should fix the totalling problem,  i do not anticipate issues with monetary use then,  although it may not be merged but it may replace your workarounds!  what do you think?18:25
tgnitthere may still be issues but it should do the minimum function expected18:25
bkuhntgnit: Yes, I think it will not be merged.  But, as I said, it's worth submitting the pull request, and let johnw formally reject it.  That will create a formal record in Github of what we tried and why johnw  didn't accept it.  This will be useful for people who want to use the patch separately.18:26
bkuhntbm and I may use it ourselves, even. :)18:27
bkuhnI have not run Conservancy's books against your patch -- I was waiting for it to be merged.18:27
bkuhnBut I'll try once it's rejected and see what happens. :)18:27
tgnitbkuhn: nice to know that  :)18:28
bkuhntgnit: at the moment, though, I have written work arounds in our books for all rounding errors by changing the numbers slightly18:28
bkuhni'll look for some of those and see if changing them back works with your patch, too18:29
bkuhnbut, let's see what johnw says officially first.18:29
tgnitok18:29
tbmyeah, there are more corner cases than we had considered18:54
tbmbut at least we've learned a lot about use cases18:54
*** bkuhn is now known as bkuhnIdle20:21
*** bkuhnIdle is now known as bkuhn20:46
tgnittbm: yes,  there are indeed!20:59
tgnitbkuhn:  i am looking through commits in src/  , for many big changes there are already regression tests.  what kind should i look for?21:00
tgnittbm, johnw made sure they add enough tests for their fixes.21:01
tbmdo ls -l under tests/*21:08
tbmand you'll see some empty files - size 021:08
tgnitok21:09
tgnitin baseline21:12
tgnitopt-cleared-format21:13
tgnitopt-date format21:13
tgnittbm: almost 14 tests21:14

Generated by irclog2html.py 2.12.1 by Marius Gedminas - find it at mg.pov.lt!