[02:46] <mwhudson> did that garbo job for backfilling product.vcs finish yet?
[02:56] <blr> mwhudson: it has yep
[02:57] <mwhudson> awesome
[02:58] <mwhudson> so as far as we know, we should be able to remove the special casing from the go tool?
[02:58] <blr> mwhudson: I would imagine so, yes :)
[03:01] <mwhudson> cool
[03:04]  * mwhudson tries to find a go project on lp with >1 series
[03:07] <mwhudson> oh poop
[03:07] <mwhudson> go get launchpad.net/project/series/sub/directory is documented as working
[03:08] <blr> mwhudson: sorry, don't follow...
[03:08] <mwhudson> as is launchpad.net/~user/project/branch
[03:09] <mwhudson> blr: basically you can feed a subdirectory of the branch to go get and it knows to get the whole branch and then only build the subdir
[03:09] <mwhudson> i assume
[03:15] <blr> mwhudson: is github special cases somehow to handle that as well?
[03:15] <blr> s/cases/cased/
[03:15] <mwhudson> yes, looks like it
[03:15] <mwhudson> bah
[03:17] <mwhudson> i guess i could cook up a re so the existing behaviour is preserved when there is trailing stuff
[03:24] <mwhudson> maybe
[03:28] <mwhudson> blr: can you link me to a project that uses git?
[03:28] <blr> mwhudson: https://launchpad.net/~blr/+git/mushrooms-russia-history
[03:29] <blr> oops
[03:29] <blr> no that's no good
[03:29] <mwhudson> that's like a junk branch?
[03:30] <blr> https://launchpad.net/germinate ?
[03:30] <blr> yes that one's git
[03:30] <blr> mwhudson: right, not a project
[03:33] <mwhudson> blr: i bet you haven't thought at all about what the go import path for a project hosted in git on lp should be :)
[03:35] <mwhudson> blr: i just said this: https://github.com/golang/go/issues/11436#issuecomment-121124410
[03:35] <blr> mwhudson: have I missed something? go-import should render for git projects too
[03:35] <mwhudson> blr: for git projects too, but what about git repos that are not the repo for a project?
[03:35] <mwhudson> i guess that's a rarer thing than the bzr case, given the nature of git
[03:36] <blr> mwhudson: right, in this case you're only going to get the default repo for the project or series
[03:51] <blr> mwhudson: how often is that used in practice? the sub-directory feature
[03:51] <mwhudson> blr: i have no idea
[03:52] <mwhudson> i suspect not often
[07:31] <rpadovani> blr, cjwatson thannk you very much, I start to take a look at it :-) I'm not good (yet) at writing tests, so maybe it takes a bit. Thanks for pointing me to the right direction
[07:38] <cjwatson> Sure.  Let us know if you need help.
[07:51] <blr> mwhudson: is there anything more we can do on the launchpad side? I see russ cox replied.
[09:00] <rpadovani> sorry to bother you, but I'm a bit lost writing the test. The general logic is ready and it does what I think it should do, but I have some problems forgint the href for the revision. Atm I've something like this: revision_link = '%s/revision/%s' % (target_identity, self.arbitrary_revisions[2]), where target_identity is bmp.merge_target.identity. It produces something similar to the final result: http://paste.ubuntu.com/11876524/ The
[09:00] <rpadovani> problem is I need to replace the starting of the link with http://bazaar.launchpad.net and with http://git.launchpad.net. I don't understand if there is a way to have the link from some variable or I need to create it form the merge_target looking if it starts with lp:// or with ~
[09:00] <rpadovani> here the code I wrote so far: http://paste.ubuntu.com/11876534/
[09:07] <cjwatson> Neither of those is correct for the link.  You'll want to construct something based on self.context.getCodebrowseUrl().
[09:10] <cjwatson> Something like self.context.getCodebrowseUrl("revision/%s" % quote_plus(revision)) should do the job for Bazaar.  For Git it'd be more like "%s/commit/?id=%s" % (self.context.getCodebrowseUrl(), quote_plus(revision))
[09:11] <cjwatson> Oh, this is from a merge proposal, so self.context.merge_target rather than self.context there
[09:12] <cjwatson> And in fact in the Git case it'll want to be self.context.target_git_repository, as GitRef.getCodebrowseUrl isn't appropriate here
[09:14] <rpadovani> okay, thanks. How can I check if the test is running agains git or bzr?
[09:15] <cjwatson> What class is this test in?
[09:15] <rpadovani> or should I create two tests, one under class TestBranchMergeProposalMergedViewGit and the other under class TestBranchMergeProposalMergedViewBzr?
[09:16] <rpadovani> TestBranchMergeProposalMergedViewMixin atm
[09:17] <cjwatson> You *can* create two tests, but I would suggest instead adding a method to each of TestBranchMergeProposalMergedView{Bzr,Git} that returns the expected link text for a revision.  See how test_initial_values is implemented for comparison.
[09:17] <cjwatson> Then the test in TestBranchMergeProposalMergedViewMixin can be generic.
[09:20] <rpadovani> okay, thanks for the ideas, I take a look
[09:50] <rpadovani> have to study now, I'll continue tonight or tomorrow, thanks for suggestions :-)
[12:17]  * cjwatson glares at the swamp of bug notifications
=== rpadovani_ is now known as rpadovani
=== igitoor_ is now known as igitoor
[20:39] <blr> morning
[21:19] <mwhudson> blr: i don't think so, i have woken up with a cold and my brain is full of cotton wool but i'll try to follow up today at some point
[21:20] <blr> mwhudson: ugh, sorry to hear that. no worries
[21:31] <rpadovani> After I spent an afternoon studying about DFA and NFA and how to transform one in the other, it's time to write a test for my branch :D So, I was trying to forge the url where I should point the link to. cjwatson suggested me to the test under TestBranchMergeProposalMergedViewMixin, and then make two little helper functions under TestBranchMergeProposalMergedViewBzr and TestBranchMergeProposalMergedViewGit. Let's focus on the first
[21:31] <rpadovani> one atm: so I created two versions of this helper function, but they don't work. Any hint on which I'm missing? I'm sure it's something very stupid...
[21:31] <rpadovani> http://paste.ubuntu.com/11879809/
[21:37] <blr> rpadovani: bmp does not have a 'context' attribute, I think you just want bmp.merge_source
[21:38] <blr> rpadovani: rather than using pastebin, it might be better to make a merge proposal in the 'work in progress state'
[21:39] <rpadovani> blr, well, I need to link to the target where the branch has been merged, so bmp.merge_target. But with bmp.merge_target I've only the name of the branch, not the address of the code (that is what I need)
[21:39] <rpadovani> blr, ok, I'll do, then the commit log for the branch will be a mess, but if it's okay for you it is also for me :-)
[21:49] <rpadovani> blr, okay, I pushed the test I wrote so far here: https://code.launchpad.net/~rpadovani/launchpad/link-revision-merged-mp/+merge/264654
[21:50] <rpadovani> if you have some time to take a look thanks so much :-)
[21:50] <rpadovani> meanwhile I take a look to the *real* code
[21:51] <blr> thanks rpadovani, someone should be able to have a look today
[21:51] <rpadovani> thanks to you guys for your time
[23:20] <wgrant> blr: Could you have a look at https://bugs.launchpad.net/launchpad/+bug/1474592? Should be a quick fix in two templates or two views, plus a test for each.
[23:20] <mup> Bug #1474592: golang_import_spec needs to check that the branch is accessible <branches> <privacy> <regression> <Launchpad itself:Triaged> <https://launchpad.net/bugs/1474592>
[23:20] <blr> sure
[23:21] <wgrant> The branch link on Product:+index and ProductSeries:+index is already guarded (possibly with a /required:launchpad.View, or possibly in the view itself)
[23:22] <blr> wgrant: worth a browser test as well?
[23:23] <cjwatson> Right, a view test would go in browser-land.
[23:24] <cjwatson> (Unless it fits with something in stories and is too hard to put elsewhere, which sin I've committed a few times.)
[23:26] <cjwatson> wgrant: Is there a way to add a person to a team in a test without triggering TestMailer.send to the added person?
[23:26] <cjwatson> I have nefarious reasons.
[23:27] <wgrant> cjwatson: Um, you could possibly fixture out the ITeamMembership subscribers, or create a TeamMembership manually and not fire the event.
[23:28] <wgrant> TeamMembershipSet.new is illegal, but it works.
[23:33] <cjwatson> Maybe a fixture is the right thing.
[23:33] <cjwatson> Or maybe I should stop trying to test this in an existing doctest ...
[23:34]  * beuno passes cjwatson a can of gasoline and a matchbook, winks and points at LP doctests
[23:35] <cjwatson> beuno: Wouldn't be the first I've committed arson against
[23:35] <cjwatson> (the first doctest, I should clarify!)
[23:35] <cjwatson> (oh hi nice mr policeman)
[23:35] <blr> trying not to grow the number of doc-tests at least
[23:36] <beuno> I still have nightmares about 6 hour test suites, PQM and getting all levels of agree to pass at the same time
[23:37] <cjwatson> It's not been that bad for a while, fortunately
[23:38] <cjwatson> Thank goodness
[23:39] <beuno> my last commits were around the time the test suite got pararellised, enough to have ended up with a 300 USD AWS bill because the tool didn't shut down instances properly, but not where the cloud bit was centralised
[23:40] <cjwatson> When I started ec2test was still a thing but there'd been enough mistakes made for there to be prominent warnings to check that shutdown had worked
[23:43]  * beuno shivers