| StevenK | wgrant: So, JavaScript and microservices? | 05:48 |
|---|---|---|
| wgrant | StevenK: Have you repushed the JS branch? | 05:48 |
| StevenK | wgrant: Yeah, the .set() changes have been up since I started on destroying PF, I've just forgotten to ping | 05:50 |
| wgrant | Ah | 05:52 |
| wgrant | StevenK: Have you tested person names with special characters such as .? | 05:55 |
| wgrant | And some of that code still uses appendChild excessively, which makes it pretty unreadable and slow when compared to code that creates the complex structure in a single Y.Node.create | 05:55 |
| wgrant | (eg. the bit under draft === true) | 05:56 |
| wgrant | I think it creates a <tr><td></td><td><td><div><span></span></div></div></td></tr>, but that would be much clearer if created as a single object. | 05:57 |
| StevenK | wgrant: Converting that bit to using .set() almost forced my brain out of my ears to begin with | 05:57 |
| wgrant | There were at least two mistakes in there. | 05:58 |
| StevenK | wgrant: One two many <td>'s, and there's a missing <div> | 05:58 |
| StevenK | One too, even | 05:58 |
| wgrant | StevenK: You only need to set two IDs, which should be pretty trivial. | 05:58 |
| StevenK | wgrant: Sure, but I want the outer div | 05:59 |
| wgrant | node.one('tr > td > div') or use a class | 05:59 |
| StevenK | Hm, yeah | 05:59 |
| StevenK | wgrant: So newrow = Y.Node.create('{everything}'); and then two lines to call .one().set() ? | 06:00 |
| wgrant | StevenK: I think that's a lot clearer, yeah | 06:01 |
| wgrant | You can see the structure | 06:01 |
| wgrant | And easily see what dynamic bits you're adding | 06:01 |
| wgrant | And it's faster. | 06:01 |
| StevenK | wgrant: Let me polish off dealing with this e-mail, and I'll change draft === true | 06:02 |
| StevenK | wgrant: http://pastebin.ubuntu.com/6095769/ | 06:24 |
| wgrant | StevenK: That looks superior. | 06:28 |
| StevenK | Hmmm, newone.one('tr') is null :-( | 06:31 |
| wgrant | StevenK: Isn't that the node itself? | 06:36 |
| wgrant | So finding a tr child is not going to be terribly effective. | 06:36 |
| wgrant | s/child/descendant/ | 06:36 |
| StevenK | Yeah, I came to that realisation about 45 seconds ago | 06:36 |
| StevenK | wgrant: Still not sure what to do about dots in names | 06:43 |
| wgrant | StevenK: dots are fine in IDs, but then you're using CSS selector syntax to find them, which will fail. | 06:46 |
| StevenK | So I should switch to person.id ? | 06:46 |
| wgrant | We don't usually expose person IDs. | 06:47 |
| wgrant | That's one possibility, but you could also just not use Node.one() to find them | 06:47 |
| StevenK | And do what instead? | 06:48 |
| wgrant | Fetch them directly by ID, rather than a CSS selector under a node | 06:49 |
| wgrant | Recall that IDs are globally unique. | 06:49 |
| stub | No quoting or escaping mechanism? | 06:49 |
| wgrant | One could also quote them, but it's not necessary here. | 06:49 |
| StevenK | Hmm | 06:50 |
| Ursinha | wgrant, should I move all checks in publishRosettaTranslations to RosettaTranslationsUpload or moving away the part that creates the job is enough? | 07:18 |
| wgrant | Ursinha: I think it makes sense to move the whole lot, for consistency. | 07:19 |
| Ursinha | wgrant, right. | 07:19 |
| wgrant | 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:20 |
| Ursinha | 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 |
| Ursinha | instead of moving the checks to RosettaTranslationsCustom, I mean | 07:24 |
| Ursinha | in the end it's your call anyway | 07:29 |
| Ursinha | :) | 07:29 |
| wgrant | Ursinha: Sorry, let me see. | 07:33 |
| wgrant | 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 |
| wgrant | Since we don't need the extra tempfile/cleanup behavior that _publishCustom provides | 07:36 |
| Ursinha | 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:38 |
| wgrant | 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 |
| wgrant | But I don't care too much. | 07:39 |
| Ursinha | 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 |
| wgrant | _publishCustom doesn't do sanity checks, though | 07:40 |
| Ursinha | indeed | 07:40 |
| wgrant | It just prepares the temporary file in a generic way, without doing anything type-specific. | 07:41 |
| Ursinha | yeah, the sanity checks are all in CustomUpload.process... | 07:41 |
| Ursinha | okay, I'll move it all :) | 07:41 |
| wgrant | Sounds good. | 07:42 |
| Ursinha | wgrant, should I also add tests for this change in lp.soyuz.tests.test_distroseriesqueue_rosetta_translations.py? | 09:19 |
| cjwatson | IIRC the archivepublisher tests are unit tests and the soyuz.tests.test_distroseriesqueue tests are kind of integration tests. | 09:26 |
| Ursinha | right | 09:40 |
| === frankban__ is now known as frankban | ||
| wgrant | Ursinha: I believe cjwatson is correct. The tests for the other custom uploads aren't a completely terrible example, IIRC | 10:21 |
| cjwatson | Yeah, I think I beat them into shape in the last big refactoring | 10:22 |
| cjwatson | Looks like I converted them from doctests | 10:22 |
| wgrant | Yeah | 10:23 |
| wgrant | They used to be one or two doctests each | 10:23 |
| === frankban__ is now known as frankban | ||
| === frankban_ is now known as frankban | ||
| === frankban_ is now known as frankban | ||
| === lifeless_ is now known as lifeless | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!