| === heroux_ is now known as heroux | ||
| blr | wgrant: do you see anything obviously wrong with the validation or call to getByPath on l.829? https://code.launchpad.net/~blr/launchpad/ui-project-setbranch/+merge/259069 | 02:03 |
|---|---|---|
| blr | wgrant: user 'kit' owns 'gitrepository-100046' yet getUtility(IGitRepositorySet).getByPath(kit, 'gitrepository-100046') is None (have tried with git_identity and other forms as well) | 02:04 |
| wgrant | blr: getByPath takes a user for privilege checking, and a full repository path. | 02:05 |
| wgrant | eg. the user should enter ~kit/project | 02:05 |
| wgrant | Or ~kit/project/+git/some-non-default-repo | 02:05 |
| blr | wgrant: is the interface comment misleading? | 02:07 |
| blr | s/comment/docstring/ | 02:07 |
| wgrant | blr: Looks fine to me. It specifies the valid forms. | 02:08 |
| wgrant | What's confusing about it? | 02:08 |
| blr | '~kit/product-name-100049/+git/gitrepository-100046' returns a repo, however '~kit/+git/gitrepository-100046' does not | 02:09 |
| wgrant | blr: The latter is a personal repo; one not related to a project. | 02:09 |
| blr | aah | 02:09 |
| blr | of course, sorry. | 02:09 |
| blr | wgrant: pity zope IField doesn't support arbitrary attributes, or more specifically 'placeholder' | 02:16 |
| blr | default isn't quite the same | 02:16 |
| wgrant | blr: Right, that's more of an attribute of the widget, I think. | 02:17 |
| wgrant | eg. a widget could use the title as a placeholder. | 02:17 |
| blr | it would be nice to have the expected form for the git repo path as a placeholder, but perhaps I'll just add it the label | 02:18 |
| blr | I suppose I could set it in the js, but that might be a little odd. | 02:18 |
| blr | will be less of an issue once we have a search modal for git. | 02:19 |
| wgrant | Yep, search will make it much better. It's also a relatively uncommon case. | 02:20 |
| blr | wgrant: from [code]/project configure hosting is still for the series, do we want that to also point to project setbranch? That would leave project/[series] as the only means of configuring a series branch | 02:33 |
| wgrant | blr: I think that's very reasonable. | 02:33 |
| blr | yep, possibly even confusing in its current state. | 02:34 |
| === danilos` is now known as danilos | ||
| === jamesh__ is now known as jamesh | ||
| cjwatson | wgrant: Nice, I didn't know about variable_name_prefix in batchnavs. | 11:17 |
| wgrant | cjwatson: I only knew about it from +members | 11:18 |
| wgrant | That's quite possibly the only other callsite. | 11:18 |
| cjwatson | wgrant: There's one other, BugTrackerSet:+index | 11:19 |
| wgrant | Ah, so there is. | 11:20 |
| cjwatson | https://bugs.launchpad.net/launchpad/+bug/1455907 suggests that the +members use doesn't work ideally, but that isn't really a batchnav problem as such. | 11:20 |
| mup | Bug #1455907: Every "Former members" batch page starts with a list of "Active members" <Launchpad itself:New> <https://launchpad.net/bugs/1455907> | 11:20 |
| wgrant | Yeah | 11:20 |
| cjwatson | Except in that maybe batchnav Next should do AJAX rather than reloading the whole page | 11:21 |
| wgrant | It can be fixed by anchors, only showing one half on subsequent batches, or using the AJAX batchnav stuff. | 11:21 |
| cjwatson | I see the FF for the last is disabled on prod | 11:23 |
| cjwatson | Indeed, everywhere | 11:23 |
| wgrant | I think AJAX batchnav is only enabled for +bugs | 11:24 |
| cjwatson | Branches too | 11:24 |
| wgrant | I'd forgotten it was even implemented for +branches until I saw the code. | 11:24 |
| cjwatson | But it's behind a disabled feature flag in both cases | 11:24 |
| wgrant | No, +bugs has been AJAXed since 2013. | 11:24 |
| cjwatson | Huh, that's interesting because I see ajax.batch_navigator.enabled mentioned in lib/lp/bugs/templates/bugs-listing-table.pt and that flag is definitely off. | 11:25 |
| wgrant | Is that the right template? | 11:26 |
| cjwatson | Perhaps not. | 11:26 |
| wgrant | Hm, I think it is. | 11:26 |
| wgrant | But there's a whole lot of custom JS for that page. | 11:26 |
| danilos | launchpad consistently times out when I attempt to comment on my MP (ajax just mentiones a server problem after ~30s, but if I go to +comment and try to submit, I get http://people.canonical.com/~danilo/tmp/lp-problem.png): any known issues? | 11:40 |
| danilos | MP in question is https://code.launchpad.net/~danilo/landscape-charm/actions/+merge/260916 | 11:41 |
| wgrant | danilos: Yep, looks like a weird postgres issue, we're investigating. | 11:42 |
| danilos | wgrant, ok, thanks (I've even tried sending an email, still hasn't shown up) | 11:42 |
| cjwatson | danilos: The mail processor will be writing to the same table, indeed. | 11:54 |
| danilos | cjwatson, yeah, I was hoping the problem was somewhere between haproxy and lp appservers :) | 11:54 |
| cjwatson | No such luck. | 11:54 |
| danilos | yeah, got it | 11:54 |
| wgrant | danilos: Should all be OK again now. | 11:58 |
| danilos | wgrant, repeated comments have gotten through, fwiw (not a big deal, just letting you guys know) | 11:59 |
| wgrant | cjwatson: Oh, I didn't realise IGitCollection handled PersonProduct etc. | 12:00 |
| cjwatson | Yup, I overachieved slightly on the adapters since it was easy. | 12:01 |
| cjwatson | Top tip to self: it's more likely that somebody will review my merge proposals if I actually remember to propose them. | 12:55 |
| === maxb_ is now known as maxb | ||
| blr | wgrant: cjwatson: any suggestions for a user response for GitTargetError that is likely to make sense to them? | 21:01 |
| blr | "repo attached to another target" may not be meaningful | 21:01 |
| blr | hmm, well I'll go with that for now given it isn't the typical case. | 21:21 |
| cjwatson | blr: I think that your form field should be using a vocabulary that only offers repositories attached to the correct target. | 21:24 |
| blr | cjwatson: I think we decided to defer the repository search modal | 21:24 |
| blr | cjwatson: so the case here is just providing the user with a git_repository_location TextLine | 21:25 |
| blr | I believe the only error paths are repo not found, and target error | 21:26 |
| blr | cjwatson: just updating the configure code link on [code]/project, but perhaps you could have a look in review if there's a better way. | 21:28 |
| blr | I'll ping you | 21:29 |
| wgrant | blr: It could still have a non-searchable vocab. | 22:20 |
| blr | wgrant: ok, so presumably we want something like ValidTargetDefault? | 23:29 |
| wgrant | blr: Any Git repository for the target is valid. | 23:30 |
| wgrant | blr: What does the branch equivalent do? It has pretty much the same rules, but we don't need searching yet. | 23:30 |
| blr | hmm branch_location doesn't have a vocabulary afaict | 23:32 |
| wgrant | blr: How's it validated? | 23:34 |
| wgrant | It's possible that it isn't; there are some cross-project relations that shouldn't exist, but they might be from branches that have been moved later. | 23:34 |
| blr | wgrant: yep only validated for presence | 23:36 |
| wgrant | blr: OK, then it might not be worth using a vocab just yet. Manually validate the target and use an error message like "Repository is in a different project." or similar? | 23:37 |
| blr | okay, that's what I have atm | 23:37 |
| wgrant | Really weird that the branch case wouldn't use a vocab already! | 23:38 |
| blr | branch_type does | 23:39 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
