--- Day changed Mon Jul 06 2020
00:12 -!- Davterra [~tralfaz@104.200.129.57] has joined ##miniscript     
00:13 -!- Davterra [~tralfaz@104.200.129.57] has left ##miniscript []     
07:15 < andytoshi> morning sanket1729 
07:16 < andytoshi> is there stuff you need me to review?
07:40 -!- dr_orlovsky [~dr-orlovs@adsl-62-167-101-63.adslplus.ch] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]     
08:08 -!- dr-orlovsky [~dr-orlovs@adsl-62-167-101-63.adslplus.ch] has joined ##miniscript     
08:31 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Ping timeout: 246 seconds]     
08:32 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined ##miniscript     
09:24 < sanket1729> andytoshi: None as of now. Would you like to release a new version of rust-miniscript?
10:07 < andytoshi> good q ... let's try to get #78 in (the parser fix) today and then we can release
10:44 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has quit [Remote host closed the connection]     
10:56 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has joined ##miniscript     
12:06 < andytoshi> hmm so, #78 fails unit tests
12:06 < andytoshi> though it's possible the tests are wrong
12:22 < sanket1729> interesting
12:22 < sanket1729> looking at it
12:28 < sanket1729> do you mean after rebasing to latest master?
12:48 < sanket1729> interesting that the unit case failing is independent from the PR
12:48 < sanket1729> looking into it
12:48 < sanket1729> looks like string -> ms error
12:51 < andytoshi> i just looked at the merge commit
12:51 < andytoshi> didn't try to rebase
12:52 < sanket1729> I thought a previous test case was failing
12:52 < sanket1729> But it was a part of that PR
12:53 < sanket1729> Yup, confirmed with sipa's website
12:53 < sanket1729> indeed the unit test is wrong
12:54 < andytoshi> nice
12:54 < andytoshi> ok, sorry, i got pulled into a phone call
12:54 < andytoshi> investigating more now
12:55 < andytoshi> what's wrong?
12:55 < andytoshi> c:pk should be pk?
12:56 < andytoshi> i wonder how these weren't fixed eariler?
12:56 < andytoshi> why does this PR trigger it?
12:57 < sanket1729> Ignore my previous explaination. Indeed the PR is correct. 
12:57 < sanket1729> Sorry
12:57 < andytoshi> yeah, just c:pk → pk and thresh_m → multi
12:57 < sanket1729> the Miniscript is correct
12:58 < andytoshi> oh i understand
12:58 < andytoshi> these are new unit tests from the PR, and they just didn't have the latest language changes
12:59 < andytoshi> confirmed that the test fails before the changes in th PR
13:01 < sanket1729> Makes sense 
13:02 < andytoshi> ok, so i don't quite understand what he's doing with chained and_vs
13:02 < andytoshi> seems like he's changing the associativity order
13:04 < andytoshi> ok, if i remove that change then the tests fail..
13:07 < andytoshi> yeah ok, i think i get it
13:07 < andytoshi> ok, lemme see if i can push to the PR to fix the unit test
13:09 < sanket1729> is it possible to extend the same PR?
13:09 < sanket1729> I think you may have to open a new one
13:10 < andytoshi> yeah i think so
13:10 < andytoshi> it's giving me permission denied pushing to his PR
13:10 < sanket1729> It maybe a good idea to have an additional commit on top this PR?
13:10 < sanket1729> to credit the author
13:12 < andytoshi> i will give the author credit for the commit
13:20 < andytoshi> https://github.com/apoelstra/rust-miniscript/pull/106
13:20 < andytoshi> sanket1729: can you review that?
13:20 < sanket1729> yeah
13:26 < sanket1729> I don't completely follow the chaining and_v chaining
13:30 < andytoshi> i recommend removing that change and running the unit test
13:30 < andytoshi> and seein how it fails
13:41 < andytoshi> oops will fix the casing issue
13:42 < sanket1729> and a better commit message. everything else looks good.
13:44 < andytoshi> lol kk i'll fix that
13:54 < sanket1729> andytoshi: with respect to release. We need a changelog and a cargo minor version bump?
13:54 < sanket1729> anything else?
13:59 < sanket1729> andytoshi: This would also require a rebase for CI tests to clear
14:00 < sanket1729> because the PR does not have the CI fixes in the latest master
14:00 < sanket1729> the branch part of travis check
14:01 < sanket1729> I can merge without having that fixed the commit after merge would work
14:15 < andytoshi> sanket1729: i think we need to do a major version
14:15 < andytoshi> because we changed the language spec
14:16 < sanket1729> Yes     
14:16 < sanket1729> the aliasing
14:16 < andytoshi> sorry, i was driving
14:16 < andytoshi> what do you mean about CI fixes
14:16 < sanket1729> I mean, travis runs 2 builds
14:16 < sanket1729> one as merge build. and one as standalone branch build
14:17 < sanket1729> merge build runs after the merge commit which has the CI fixes
14:17 < andytoshi> what CI fixes?
14:17 < andytoshi> what PR are we talking about?
14:17 < sanket1729> The one which you just raised
14:17 < sanket1729> it was failing on branch build
14:17 < sanket1729> which was expected
14:18 < sanket1729> because it was based on master from Feb
14:18 < andytoshi> lol oops
14:18 < andytoshi> I could've rebased
14:18 < sanket1729> But, the merge commit was properly building
14:18 < andytoshi> ok, sounds good then
14:18 < andytoshi> in future it's probably best to rebase in any case
14:18 < andytoshi> because long-range merges make the git history a bit harder to follow 
14:19 < sanket1729> Sounds good
14:19 < sanket1729> I can look into final changes for release
14:20 < andytoshi> that would be awesome
14:20 < sanket1729> README/CHANGELOG and cargo major version
14:20 < andytoshi> try     
14:20 < andytoshi> git log --graph --oneline master --not 0.12.0
14:20 < sanket1729> anything else?
14:20 < andytoshi> to see all the PRs that were merged since the last release
14:20 < andytoshi> no, i think we're set to release
14:20 < sanket1729> wow, that command looks great
14:21 < andytoshi> yeah i was pretty happy to discover --graph :)
14:21 < andytoshi> next step would be to PR to liquid functionary code to use the new version (but we have to get stevenroose network compat changes in first)
14:21 < andytoshi> when we do that probably we'll discover a bunch of API rough points
14:22 < sanket1729> Can you also address some of the private key concerns in #93?
14:23 < andytoshi> oh right
14:23 < andytoshi> yeah one sec
14:30 < andytoshi> ok replied to sgeisler 
14:30 < andytoshi> on github
15:01 -!- instagibbs [~instagibb@pool-71-178-191-230.washdc.fios.verizon.net] has quit [Ping timeout: 256 seconds]     
15:02 < sanket1729> made a PR
15:33 -!- instagibbs [~instagibb@pool-71-178-191-230.washdc.fios.verizon.net] has joined ##miniscript     
16:02 < andytoshi> whew 1.0.0 :)
16:02 < andytoshi> maybe we should also pull it under the rust-bitcoin org now
16:06 < sanket1729> sure :)
16:06 < andytoshi> nice
16:07 < andytoshi> so .. i think we should try as much as possible to put the simplicity-miniscipt into rust-simplicity rather than rust-miniscript
16:07 < andytoshi> e.g the "compiler" and the lift impl
16:13 < sanket1729> agreed
16:49 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has quit [Remote host closed the connection]     
17:10 -!- Davterra [~tralfaz@c-73-221-225-225.hsd1.wa.comcast.net] has joined ##miniscript     
17:11 -!- Davterra [~tralfaz@c-73-221-225-225.hsd1.wa.comcast.net] has left ##miniscript []     
19:32 -!- Davterra [~tralfaz@178.128.106.205] has joined ##miniscript     
19:37 -!- Davterra [~tralfaz@178.128.106.205] has left ##miniscript []     
20:21 < jonatack> bravo!