[05:58] One too, even [05:58] StevenK: You only need to set two IDs, which should be pretty trivial. [05:59] wgrant: Sure, but I want the outer div [05:59] node.one('tr > td > div') or use a class [05:59] Hm, yeah [06:00] wgrant: So newrow = Y.Node.create('{everything}'); and then two lines to call .one().set() ? [06:01] StevenK: I think that's a lot clearer, yeah [06:01] You can see the structure [06:01] And easily see what dynamic bits you're adding [06:01] And it's faster. [06:02] wgrant: Let me polish off dealing with this e-mail, and I'll change draft === true [06:24] wgrant: http://pastebin.ubuntu.com/6095769/ [06:28] StevenK: That looks superior. [06:31] Hmmm, newone.one('tr') is null :-( [06:36] StevenK: Isn't that the node itself? [06:36] So finding a tr child is not going to be terribly effective. [06:36] s/child/descendant/ [06:36] Yeah, I came to that realisation about 45 seconds ago [06:43] wgrant: Still not sure what to do about dots in names [06:46] StevenK: dots are fine in IDs, but then you're using CSS selector syntax to find them, which will fail. [06:46] So I should switch to person.id ? [06:47] We don't usually expose person IDs. [06:47] That's one possibility, but you could also just not use Node.one() to find them [06:48] And do what instead? [06:49] Fetch them directly by ID, rather than a CSS selector under a node [06:49] Recall that IDs are globally unique. [06:49] No quoting or escaping mechanism? [06:49] One could also quote them, but it's not necessary here. [06:50] Hmm [07:18] wgrant, should I move all checks in publishRosettaTranslations to RosettaTranslationsUpload or moving away the part that creates the job is enough? [07:19] Ursinha: I think it makes sense to move the whole lot, for consistency. [07:19] wgrant, right. [07:20] The custom upload types are fundamentally different in that rosetta translations don't get extracted, but we should still keep the code as similar as is practical. [07:24] wgrant, so I could move it to a _publishRosettaCustom or something, should have the same effect and will keep publishRosettaTranslations consistent with the rest [07:24] instead of moving the checks to RosettaTranslationsCustom, I mean [07:29] in the end it's your call anyway [07:29] :) [07:33] Ursinha: Sorry, let me see. [07:36] Ursinha: Ah, I see. We obviously can't use _publishCustom for this, as that assumes we're extracting a file. So I'd just move the contents of publishRosettaTranslations to new function in lp.archivepublisher.rosetta_translations and call that directly in publishRosettaTranslations. [07:36] Since we don't need the extra tempfile/cleanup behavior that _publishCustom provides [07:38] wgrant, I thought we could avoid having to import a lot of things to do the checks in lp.archivepublisher.rosetta_translations when we already have it all in queue.py :) I was suggesting move the checks to a _publishRosettaCustom instead [07:39] Ursinha: I think it's probably still slightly nicer to have it encapsulated in the other module like the rest, rather than inline in queue.py. [07:39] But I don't care too much. [07:40] because _publishCustom prepares and runs the process_* methods, so I thought it would make sense to have the checks before process_rosetta_translations as well [07:40] _publishCustom doesn't do sanity checks, though [07:40] indeed [07:41] It just prepares the temporary file in a generic way, without doing anything type-specific. [07:41] yeah, the sanity checks are all in CustomUpload.process... [07:41] okay, I'll move it all :) [07:42] Sounds good. [09:19] wgrant, should I also add tests for this change in lp.soyuz.tests.test_distroseriesqueue_rosetta_translations.py? [09:26] IIRC the archivepublisher tests are unit tests and the soyuz.tests.test_distroseriesqueue tests are kind of integration tests. [09:40] right === frankban__ is now known as frankban [10:21] Ursinha: I believe cjwatson is correct. The tests for the other custom uploads aren't a completely terrible example, IIRC [10:22] Yeah, I think I beat them into shape in the last big refactoring [10:22] Looks like I converted them from doctests [10:23] Yeah [10:23] They used to be one or two doctests each === frankban__ is now known as frankban === frankban_ is now known as frankban === frankban_ is now known as frankban === lifeless_ is now known as lifeless