[01:37] <cjwatson> tsimonq2: you have review comments
[01:37] <cjwatson> (thanks
[01:37] <cjwatson> )
[01:38] <tsimonq2> cjwatson: Many thanks.
[01:42] <tsimonq2> cjwatson: To be fair with the whole whitespace thing, it was a wild shot in the dark, I didn't remember how to do that particular element at all; so thanks.
[01:44] <cjwatson> Not a problem
[02:05] <tsimonq2> cjwatson: So about using "==" over "is", I think Steve prefers the opposite, and that's already approved, so for the sake of consistency unless you feel strongly about it, I'll keep it consistent ;)
[02:05] <tsimonq2> cjwatson: Even though that part does get removed in newer commits, I think it's just worth a quick note. :)
[02:11] <cjwatson> tsimonq2: I don't feel that strongly about it, and looking at the enum documentation I see that it explicitly guarantees that object identity comparison will work, so I guess it's OK
[02:13] <tsimonq2> cjwatson: ok
[02:26] <tsimonq2> cjwatson: Question: would it be acceptable to do something like `git rev-parse --symbolic-full-name bionic@{upstream}` and then just strip the `/`s and just get the third item from that (in this case, just the string "origin"), or might that be dynamic enough for it not to work?
[02:27] <tsimonq2> cjwatson: Or do you know of a better way to extract that string besides just doing a .split("/")[2]?
[02:29] <cjwatson> tsimonq2: There may be a better way (git surely knows how to do it internally), but I couldn't find it.  .split("/")[2] should be OK for now.
[02:29] <tsimonq2> cjwatson: Alright.
[03:03] <tsimonq2> cjwatson: So I've hit a dead end when you have a minute... the following code is what replaced the string replacement code I had before, with all other parts staying the same:
[03:03] <tsimonq2> lp_git_repo = options.launchpad.git_repositories.getByPath(
[03:03] <tsimonq2>                       path=urlparse(remote).path.lstrip("/"))
[03:04] <tsimonq2> I confirmed that urlparse(remote).path.lstrip("/") is in fact a string and it's equal to "~tsimonq2/ubuntu-seeds/+git/lubuntu"
[03:04] <tsimonq2> However, I'm getting thrown a 400 from Launchpad, more specifically: lazr.restfulclient.errors.BadRequest: HTTP Error 400: Bad Request
[03:06] <tsimonq2> I guess I'm wondering if this is PEBKAC or if this is Launchpad.
[03:16] <cjwatson> In [1]: lp.git_repositories.getByPath(path="~tsimonq2/ubuntu-seeds/+git/lubuntu")
[03:16] <cjwatson> Out[1]: <git_repository at https://api.launchpad.net/devel/~tsimonq2/ubuntu-seeds/+git/lubuntu>
[03:16] <cjwatson> So I'm not seeing the problem.  Is the code above where the exception is being raised?
[03:18] <cjwatson> You'd get a 404 from trying to set default_branch if there's no such branch in the repository yet as far as LP knows; if you've only just pushed it then beware that it takes a short but non-zero amount of time for LP to scan the repository, so you might need a retry/backoff loop to handle that.  Not sure where you'd get a 400 though.
[03:19] <cjwatson> (Or rather from trying to .lp_save() after setting .default_branch)
[03:20] <tsimonq2> Right, it does occur there, and that could certainly be a possibility because in the Python console it works fine if I take it step by step keeping variable names and values
[03:20] <cjwatson> You should print the content of the exception to find out more detail.
[03:21] <tsimonq2> Would this be verbose enough? https://paste.ubuntu.com/26401875/
[03:23] <cjwatson> I'm not seeing why that would return 400.  getByPath should just return None if there's no match.
[03:24] <tsimonq2> Let me push my work in progress code real quick...
[03:25] <cjwatson> Try putting "import httplib2; httplib2.debuglevel = 1" near the top of your code to dump the actual request that's happening.  (Beware that it will contain your OAuth secrets, so will need to be sanitised and not just pastebinned.)
[03:25] <tsimonq2> Sure, while I'm doing that, I pushed my code to the MP.
[03:25] <cjwatson> It should have   send: 'GET /devel/+git?path=%22%7Etsimonq2%2Fubuntu-seeds%2F%2Bgit%2Flubuntu%22&ws.op=getByPath HTTP/1.1\r\nHost: api.launchpad.net\r\n   in it
[03:26] <cjwatson> (remote_git_repository rather than remote_git_branch, pedantically.)
[03:27] <cjwatson> I see no obvious problems right now ...
[03:29] <tsimonq2> Right: send: 'GET /devel/+git?path=%22%7Etsimonq2%2Fubuntu-seeds%2F%2Bgit%2Flubuntu%5Cn%22&ws.op=getByPath HTTP/1.1\r\nHost: api.launchpad.net\r
[03:30] <tsimonq2> Then the next line is: reply: 'HTTP/1.1 400 Bad Request\r\n'
[03:30] <cjwatson> What's that %5Cn doing in the encoding?
[03:30] <cjwatson> Trailing literal "\n"
[03:31] <cjwatson> remote_git_branch (or remote_git_repository) should call .rstrip("\n") on the result of subprocess.check_output before returning it
[03:31] <tsimonq2> Yep.
[03:31] <tsimonq2> That was it.
[03:32] <tsimonq2> Cool. :)
[03:32] <tsimonq2> Thanks cjwatson
[03:32] <cjwatson> No problem
[03:32] <cjwatson> remote_branch doesn't have this problem because .splitlines() deals with it
[03:32] <tsimonq2> Oh, right.
[03:34] <tsimonq2> cjwatson: Now the MP should be updated with my latest commits that (I think) address all of your comments.
[03:37] <cjwatson> I have some remaining whitespace nits, but can fix those up after merging rather than making you do them.
[03:37] <cjwatson> +def remote_git_repository(source, srcbranch):
[03:37] <cjwatson> + remote = remote_git_repository(dest, options.source_series)
[03:37] <cjwatson> that looks weird
[03:38] <cjwatson> but apparently correct, just surprising naming
[03:39] <tsimonq2> Yeah, I think in that specific part of it, I sorta ran out of ideas (and was hoping you'd suggest alternatives if you had fitting ones), but it works. :P
[03:39] <cjwatson> not for this branch, but do we actually want to rename the whole git repository on disk from series to series?  I know there's legacy stuff to handle, but I'd have expected the Lubuntu seed repository to just be called "lubuntu" on disk, not lubuntu.bionic etc.
[03:40] <tsimonq2> I did that for the sake of backwards compatibility for any bit of tooling that happens to depend on Lubuntu's bionic seed being at lubuntu.bionic
[03:40] <tsimonq2> Of course given assurance that this won't break anything, I can refactor that.
[03:41] <cjwatson> the only such thing I know of is for the published seed branches on people.c.c/~ubuntu-archive/seeds/
[03:41] <cjwatson> for branch-seeds' purposes I think there'd be no breakage
[03:42] <tsimonq2> Alright, that's fine by me then. Would you like me to propose a follow-up MP or would you just like to do it?
[03:46] <cjwatson> tsimonq2: the former :)
[03:46] <cjwatson> tsimonq2: I've merged your branch and applied a few follow-up tweaks - thanks!
[03:46] <tsimonq2> cjwatson: Awesome, thanks.
[03:47] <cjwatson> tsimonq2: Oh, it occurs that for symmetry maybe you should have an lp_git_repository helper, just like lp_branch.
[03:47] <cjwatson> which is extremely similar in structure ...
[03:48] <tsimonq2> cjwatson: I think it wouldn't be used because lp_branch seems to only be called in the Bazaar section of the code.
[03:49] <tsimonq2> cjwatson: Er, maybe you're saying that I should move the similar code up to its own separate function?
[03:50] <tsimonq2> What would be the advantages to doing that verus keeping it how it is?
[03:53] <cjwatson> tsimonq2: It's just odd to have it factored out to a separate function for bzr but not for git.
[03:54] <cjwatson> No good reason for that not to be symmetric.
[03:55] <tsimonq2> Right, but I could either put the contents of that function in the Bazaar section of the code and remove the function, or add an additional Git function. I'm thinking the former of which is borderline redundancy, but it's up to you because you have commit access. :)
[03:55] <tsimonq2> s/former/latter/
[03:58] <cjwatson> I don't care very deeply either way.
[03:58] <tsimonq2> Alright.
[04:00] <tsimonq2> cjwatson: Before I go ahead and commit this, would something like this be acceptable for what we talked about earlier? http://paste.ubuntu.com/26401984/
[04:01] <tsimonq2> Because in order to do the VCS detection, you need to have the directory, so that would be a catch 22 if we made it VCS-dependent.
[04:13] <blackboxsw> :w
[04:14] <tsimonq2> (<3 Vim)
[05:42] -queuebot:#ubuntu-release- New: accepted kylin-display-switch [amd64] (bionic-proposed) [1.0.1-0ubuntu1]
[05:42] -queuebot:#ubuntu-release- New: accepted libpath-iter-perl [amd64] (bionic-proposed) [0.2-1]
[05:42] -queuebot:#ubuntu-release- New: accepted pgq-node [amd64] (bionic-proposed) [3.2.5-1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-devtools [i386] (bionic-proposed) [1.13.4-1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-rhandsontable [amd64] (bionic-proposed) [0.3.4+dfsg-1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-utf8 [amd64] (bionic-proposed) [1.1.3-1]
[05:42] -queuebot:#ubuntu-release- New: accepted kylin-display-switch [i386] (bionic-proposed) [1.0.1-0ubuntu1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-devtools [amd64] (bionic-proposed) [1.13.4-1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-shinydashboard [amd64] (bionic-proposed) [0.6.1-1]
[05:42] -queuebot:#ubuntu-release- New: accepted papirus-icon-theme [amd64] (bionic-proposed) [20171223-1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-utf8 [i386] (bionic-proposed) [1.1.3-1]
[05:42] -queuebot:#ubuntu-release- New: accepted r-cran-psych [amd64] (bionic-proposed) [1.7.8-1]
[06:19] -queuebot:#ubuntu-release- New binary: r-cran-pillar [amd64] (bionic-proposed/universe) [1.0.1-1] (no packageset)
[06:20] -queuebot:#ubuntu-release- New binary: r-cran-broom [amd64] (bionic-proposed/universe) [0.4.3-1] (no packageset)
=== s8321414_ is now known as s8321414
=== cpaelzer_ is now known as cpaelzer
[08:28] <cjwatson> tsimonq2: "%s" % collection could just be spelled collection, but
[08:28] <cjwatson> tsimonq2: ... seems reasonable
[12:04] <tsimonq2> cjwatson: ack, thanks
[16:08] -queuebot:#ubuntu-release- Unapproved: grub2 (bionic-proposed/main) [2.02-2ubuntu3 => 2.02-2ubuntu3] (core)
[16:43] -queuebot:#ubuntu-release- Unapproved: accepted google-apputils-python [source] (xenial-proposed) [0.4.1-1ubuntu2.16.04.1]
[16:44] -queuebot:#ubuntu-release- Unapproved: accepted google-apputils-python [source] (artful-proposed) [0.4.1-1ubuntu2.17.10.1]
[16:54] <cyphermox> could someone please reject grub2 and grub2-signed from artful unapproved queue?
[17:07] -queuebot:#ubuntu-release- Unapproved: rejected grub2-signed [source] (artful-proposed) [1.85.1]
[17:07] -queuebot:#ubuntu-release- Unapproved: rejected grub2 [source] (artful-proposed) [2.02~beta3-4ubuntu8]
[17:15] -queuebot:#ubuntu-release- New binary: liburcu [i386] (bionic-proposed/main) [0.10.0-3] (ubuntu-desktop, ubuntu-server)
[17:16] <cyphermox> ta
[17:18] -queuebot:#ubuntu-release- New binary: liburcu [amd64] (bionic-proposed/main) [0.10.0-3] (ubuntu-desktop, ubuntu-server)
[17:19] -queuebot:#ubuntu-release- Unapproved: accepted python-oslo.messaging [source] (artful-proposed) [5.30.0-0ubuntu2]
[20:30] -queuebot:#ubuntu-release- Unapproved: shim-signed (artful-proposed/main) [1.32 => 1.33.1~17.10.1] (core)
[20:32] -queuebot:#ubuntu-release- Unapproved: grub2 (artful-proposed/main) [2.02~beta3-4ubuntu7 => 2.02~beta3-4ubuntu7.1] (core)
[20:33] -queuebot:#ubuntu-release- Unapproved: grub2-signed (artful-proposed/main) [1.85 => 1.85.1] (core)
[22:24] -queuebot:#ubuntu-release- Unapproved: grub2-signed (xenial-proposed/main) [1.66.15 => 1.66.16] (core)
[22:24] -queuebot:#ubuntu-release- Unapproved: grub2 (xenial-proposed/main) [2.02~beta2-36ubuntu3.15 => 2.02~beta2-36ubuntu3.16] (core)
[22:28] -queuebot:#ubuntu-release- Unapproved: shim-signed (xenial-proposed/main) [1.32~16.04.1 => 1.33.1~16.04.1] (core)
[22:52] <tsimonq2> cjwatson: Could you please let lubuntu-meta/0.88 build on {armhf,arm64} and then do a Binary NEW review of the arm64 packages?
[22:52] <tsimonq2> (or someone else if they have time :) )
[22:57] <cjwatson> tsimonq2: I've done the former; the latter is a good bit less restricted so I'll let others do that
[22:57] <tsimonq2> cjwatson: OK cool, thanks
[23:06] -queuebot:#ubuntu-release- New source: ukui-menus (bionic-proposed/primary) [1.1.1-0ubuntu1]
[23:46] <nacc> slangasek: infinity: should yakkety be moved to old-releases?
[23:46] <nacc> (noticing that zesty already has, but yakkety is still on the us archive at least)
[23:47] <tsimonq2> (oh, please, let's not do another vivid >:()
[23:47] <nacc> vivid at least was for phones, it made some sense
[23:47] <nacc> but i don't think anything is using yakkety
[23:49] <nacc> slangasek: infinity: sorry, perhaps i should have said, should yakkety's archive be moved?