| mwhudson | did that garbo job for backfilling product.vcs finish yet? | 02:46 |
|---|---|---|
| blr | mwhudson: it has yep | 02:56 |
| mwhudson | awesome | 02:57 |
| 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 :) | 02:58 |
| mwhudson | cool | 03:01 |
| * mwhudson tries to find a go project on lp with >1 series | 03:04 | |
| mwhudson | oh poop | 03:07 |
| mwhudson | go get launchpad.net/project/series/sub/directory is documented as working | 03:07 |
| blr | mwhudson: sorry, don't follow... | 03:08 |
| mwhudson | as is launchpad.net/~user/project/branch | 03:08 |
| 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:09 |
| 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:15 |
| mwhudson | i guess i could cook up a re so the existing behaviour is preserved when there is trailing stuff | 03:17 |
| mwhudson | maybe | 03:24 |
| 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:28 |
| blr | oops | 03:29 |
| blr | no that's no good | 03:29 |
| mwhudson | that's like a junk branch? | 03:29 |
| blr | https://launchpad.net/germinate ? | 03:30 |
| blr | yes that one's git | 03:30 |
| blr | mwhudson: right, not a project | 03:30 |
| 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:33 |
| 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:35 |
| blr | mwhudson: right, in this case you're only going to get the default repo for the project or series | 03:36 |
| blr | mwhudson: how often is that used in practice? the sub-directory feature | 03:51 |
| mwhudson | blr: i have no idea | 03:51 |
| mwhudson | i suspect not often | 03:52 |
| 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:31 |
| cjwatson | Sure. Let us know if you need help. | 07:38 |
| blr | mwhudson: is there anything more we can do on the launchpad side? I see russ cox replied. | 07:51 |
| 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:00 |
| cjwatson | Neither of those is correct for the link. You'll want to construct something based on self.context.getCodebrowseUrl(). | 09:07 |
| 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:10 |
| cjwatson | Oh, this is from a merge proposal, so self.context.merge_target rather than self.context there | 09:11 |
| 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:12 |
| rpadovani | okay, thanks. How can I check if the test is running agains git or bzr? | 09:14 |
| 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:15 |
| rpadovani | TestBranchMergeProposalMergedViewMixin atm | 09:16 |
| 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:17 |
| rpadovani | okay, thanks for the ideas, I take a look | 09:20 |
| rpadovani | have to study now, I'll continue tonight or tomorrow, thanks for suggestions :-) | 09:50 |
| * cjwatson glares at the swamp of bug notifications | 12:17 | |
| === rpadovani_ is now known as rpadovani | ||
| === igitoor_ is now known as igitoor | ||
| blr | morning | 20:39 |
| 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:19 |
| blr | mwhudson: ugh, sorry to hear that. no worries | 21:20 |
| 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:31 |
| blr | rpadovani: bmp does not have a 'context' attribute, I think you just want bmp.merge_source | 21:37 |
| blr | rpadovani: rather than using pastebin, it might be better to make a merge proposal in the 'work in progress state' | 21:38 |
| 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:39 |
| 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:49 |
| 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:50 |
| blr | thanks rpadovani, someone should be able to have a look today | 21:51 |
| rpadovani | thanks to you guys for your time | 21:51 |
| 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:20 |
| 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:21 |
| blr | wgrant: worth a browser test as well? | 23:22 |
| cjwatson | Right, a view test would go in browser-land. | 23:23 |
| cjwatson | (Unless it fits with something in stories and is too hard to put elsewhere, which sin I've committed a few times.) | 23:24 |
| 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:26 |
| wgrant | cjwatson: Um, you could possibly fixture out the ITeamMembership subscribers, or create a TeamMembership manually and not fire the event. | 23:27 |
| wgrant | TeamMembershipSet.new is illegal, but it works. | 23:28 |
| 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:33 |
| * beuno passes cjwatson a can of gasoline and a matchbook, winks and points at LP doctests | 23:34 | |
| 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:35 |
| beuno | I still have nightmares about 6 hour test suites, PQM and getting all levels of agree to pass at the same time | 23:36 |
| cjwatson | It's not been that bad for a while, fortunately | 23:37 |
| cjwatson | Thank goodness | 23:38 |
| 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:39 |
| 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:40 |
| * beuno shivers | 23:43 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!