| === Ursinha is now known as Ursinha-afk | ||
| === henninge_ is now known as henninge | ||
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: || queue: || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | hi adeuring, when you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/567922-fix-ec2-failures-after-db-devel-merge/+merge/25759 | 11:50 |
|---|---|---|
| * adeuring is looking | 11:50 | |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | noodles775: before I start to read through the diff: is it correct with > 1000 lines? | 11:55 |
| noodles775 | adeuring: yeah, 1055 lines (sorry). It's mostly brainless substitutions (buildstate->status etc), I got carried away while fixing them and forgot to check the diff. If it's a pain for you, I can split it up. | 11:58 |
| adeuring | noodles775: ok, no problem -- I just wanted to avoid to read a possibly wrong diff ;) | 11:59 |
| noodles775 | OK, I just pushed 9306 (one line change that I missed in archive-views.txt). Thanks. | 12:00 |
| === mrevell is now known as mrevell-lunch | ||
| === mrevell-lunch is now known as mrevell | ||
| adeuring | noodles775: r=me | 12:40 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| noodles775 | Thanks adeuring | 13:04 |
| === Ursinha-afk is now known as Ursinha | ||
| deryck | adeuring, I have a branch for review. | 15:17 |
| === deryck changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: - || queue: [deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | deryck: sure, I'll look | 15:17 |
| deryck | adeuring, thanks! https://code.launchpad.net/~deryck/launchpad/move-pagetests-bug-583472/+merge/25776 | 15:17 |
| adiroiban | henninge: hi. What do you suggest we should do for landing the branch for bug 525992 ? | 15:19 |
| henninge | adiroiban: merge devel and try again ... | 15:27 |
| adeuring | deryck: r=me. Thanks for this cleanup! | 15:38 |
| deryck | adeuring, thanks! | 15:39 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === deryck is now known as deryck[lunch] | ||
| adiroiban | danilos, henninge: are you in the mood for a quick UI review for bug 512133 ? | 15:46 |
| mup | Bug #512133: Make template description visible to translators <Launchpad Translations:Triaged by adiroiban> <https://launchpad.net/bugs/512133> | 15:46 |
| henninge | adiroiban: that sounds simple. Let's see the mp ;) | 15:47 |
| henninge | adiroiban: actually, maybe adeuring can do that, too. | 15:47 |
| adiroiban | henninge: I have attached a mockup http://launchpadlibrarian.net/48833845/translate-file-details.pdf | 15:47 |
| adiroiban | no code yet, as placing that information on an already busy page is not trivial | 15:48 |
| adeuring | henninge, adiroiban Im not even an apprentice for UI reviews ;) | 15:48 |
| * henninge wonders why Chromium cannot handle PDS | 15:48 | |
| henninge | adeuring, adiroiban: I overead the "UI" bit .... ;-) | 15:48 |
| adiroiban | :) | 15:48 |
| henninge | adiroiban: you will have to get a proper UI review. | 15:49 |
| henninge | adiroiban: the second box is not dismissible? | 15:50 |
| adiroiban | henninge: no | 15:50 |
| adiroiban | is the template details | 15:50 |
| henninge | and it expands when I click on more ... ? | 15:50 |
| adiroiban | you will be able to „fold” it | 15:50 |
| adiroiban | yes | 15:50 |
| henninge | I know, we talked about that at UDS | 15:50 |
| adiroiban | on „more” and on the right icon | 15:50 |
| henninge | adiroiban: I know the icon will fail UI review. | 15:51 |
| henninge | these triangles belong on the left side., | 15:51 |
| henninge | BUT | 15:51 |
| henninge | thare also the icons that you find next to the input boxes. May one of those works. | 15:51 |
| * henninge just found the second page ... ;) | 15:52 | |
| adiroiban | in the PDF, you should be able to click on „more” and it will take you to the second page | 15:53 |
| henninge | Wow! | 15:53 |
| henninge | :D | 15:53 |
| henninge | other things I see, just checking if they are deliberate: | 15:55 |
| henninge | - urls get linkified (like elsewhere in LP) | 15:55 |
| henninge | - when minimized, newlines in the text are ignored | 15:56 |
| adiroiban | henninge: ignoring newlines in minimeze is deliberate | 15:56 |
| adiroiban | to compact the text and since the minimezed details will only be one line | 15:57 |
| henninge | - I assume we will have the same thing on the template's page | 15:57 |
| henninge | adiroiban: good idea | 15:57 |
| henninge | adiroiban: I like it, apart from the green triangle. See what danilo thinks. | 15:58 |
| henninge | oh, to continue my list: | 15:58 |
| noodles775 | Looks great Adi :). Just a thought, why not have the normal (X) instead of the triangle, so that it can be dismissed in the same way? (given that clicking on 'more' will display the rest?) | 15:58 |
| gmb | adeuring, Good afternoon. Could I get a review for https://code.edge.launchpad.net/~gmb/launchpad/inline-help-for-bugwatches-2-bug-530162/+merge/25784 please? | 15:58 |
| adeuring | gmb: sure | 15:58 |
| gmb | adeuring, Thanks. | 15:58 |
| adiroiban | noodles775: the idea is that those details shoud be permanent | 15:58 |
| henninge | - The text "Developers notes" is static text, not editable. | 15:59 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: gmb || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | not just a transient notice like „did you knwo about the guidelines” | 15:59 |
| danilos | adiroiban, it looks good, I agree with henninge for most of it :) | 15:59 |
| gmb | mrevell, I've added some popup help to explain bug watch errors and give pointers for fixing them (where possible) do you have the time to review it? | 15:59 |
| adiroiban | henninge: yes. „Developers notes” is static | 15:59 |
| mrevell | gmb, I'd love to. Should I check the queue? | 15:59 |
| adiroiban | to inform translators what are all those words about | 15:59 |
| danilos | adiroiban, well, link to translation guidelines is as permanent | 15:59 |
| gmb | mrevell, The MP is here: https://code.edge.launchpad.net/~gmb/launchpad/inline-help-for-bugwatches-2-bug-530162/+merge/25784 | 15:59 |
| henninge | - the text is indented (I wonder if that is necessary) | 15:59 |
| mrevell | thanks gmb | 16:00 |
| danilos | adiroiban, it's just that once you read it there's no need for it to take up that much space on the most prominent part of the page | 16:00 |
| gmb | mrevell, The file you want to look at is lib/lp/bugs/templates/bugwatch-error-help.pt | 16:00 |
| noodles775 | oh really? I'm not sure that info/message boxes were meant to be permanent like that (although I think I have seen them used that way elsewhere... it'd be good to know where). | 16:00 |
| henninge | danilos: wasn't it that the balloon re-appears every day? | 16:00 |
| gmb | mrevell, I've created a dynamic help page rather than using the static /+help/ system. | 16:00 |
| gmb | So that I can give them links to specific things. | 16:01 |
| henninge | balloon = message box | 16:01 |
| adiroiban | danilos: adding a cookie for each POTemplate can generate lots of cookies | 16:01 |
| danilos | henninge, I don't remember the details of how the cookie is set up, but it is limited to a certain context | 16:01 |
| adiroiban | danilos: and I was thinking that the description is updated more frequent than the guidelines | 16:01 |
| danilos | adiroiban, then you should add a cookie only if it's dismissed :) | 16:02 |
| danilos | adiroiban, a session cookie then? :) | 16:02 |
| noodles775 | adiroiban: not that it's a great example, but I think the precedent for text like that is to use a drop-down like on the PPA page: https://edge.launchpad.net/~michael.nelson/+archive/pocketsphinx | 16:02 |
| noodles775 | (see "Technical details about this PPA") | 16:02 |
| adiroiban | danilos: on Chromium, session cookies last a long time | 16:02 |
| danilos | adiroiban, uhm, ok, so? | 16:02 |
| adiroiban | so they are not deleted when closing the browser | 16:03 |
| noodles775 | (another example on source page "Other versions of 'compiz' in unt..." https://edge.launchpad.net/ubuntu/+source/compiz ) | 16:03 |
| mrevell | gmb, Cool! So, the links go to the bug watch page from which the pop-up was called, is that right? | 16:03 |
| danilos | noodles775, just for clarification, this is something that might not be defined, and if it is, we want people to read it | 16:04 |
| adiroiban | and it is hard to know what a „session” means for each browser | 16:04 |
| gmb | mrevell, Yes. Also to the remote bug, where appropriate (i.e. where I'm saying things like "check that the remote bug (link) exists." | 16:04 |
| adiroiban | noodles775: thank. I will go for those icons for fold/unfold | 16:04 |
| mrevell | gmb, That is excellent. Nice one. | 16:04 |
| gmb | Ta | 16:04 |
| noodles775 | danilos: yep, I understood that, just not sure that using a msg box/balloon is something we want to do for permanent notices. | 16:04 |
| mrevell | gmb, Is it reusable or would it require more dev work for each different situation? | 16:05 |
| danilos | noodles775, I don't think this one should be permanent either :) but I am just arguing over that with adiroiban, so let's see what we come up with :) | 16:05 |
| noodles775 | danilos: great. | 16:05 |
| danilos | adiroiban, that clearly sounds like a bug in chromium then | 16:05 |
| gmb | mrevell, It's pretty context-specific. There's no back-end view to this, I'm just referencing the current context (i.e. the bug watch) in the template. | 16:06 |
| gmb | I don't think it can be easily genericised. | 16:06 |
| adiroiban | danilos: my only problem with this solution, is that if you accidentaly dismissed that notification | 16:06 |
| mrevell | Ah right. | 16:06 |
| adiroiban | it will be hard to undo your action | 16:06 |
| danilos | adiroiban, well, the same is true for anything dismissable, and I agree that's a problem, but that's a problem as much for the guidelines box, isn't it? | 16:07 |
| adiroiban | danilos: but maybe we can add a small button / text for „Show details again” | 16:07 |
| danilos | adiroiban, uhm, no :) | 16:07 |
| danilos | adiroiban, we should just show that text permanently on the pofile +details or +overview page | 16:08 |
| adiroiban | danilos: yes. same problem for guidelines box | 16:08 |
| adiroiban | danilos: ok. then the details box, will behave like the guidelines box | 16:08 |
| gmb | mrevell, Thanks for spotting the typo :). I had a hell of a time writing that sentence in a coherent manner. | 16:09 |
| danilos | adiroiban, the fact that pofile:+details page is almost useless now doesn't mean it needs to stay that way; though, getting it fixed so it resembles anything useful would be hard | 16:09 |
| gmb | Launchpad can't be described in natural language, I think... | 16:09 |
| mrevell | gmb, heh, I can imagine :) | 16:09 |
| danilos | adiroiban, the "expand" trick is nice, though :) | 16:09 |
| mrevell | gmb, I've spent the past three and a half years trying | 16:09 |
| gmb | :) | 16:09 |
| danilos | adiroiban, I'd even consider putting it in the same box for now, but basically, what we need on +translate page is a nice place to put things like "interesting things to look at" | 16:10 |
| danilos | adiroiban, btw, if you are filing a chromium bug, reference http://www.ietf.org/rfc/rfc2109.txt and section 4.3.1 where it says that cookie should be discarded when user agent exits ;) | 16:11 |
| adiroiban | danilos: yep... but now with the feature of saving tabs and state, i guess this is the desire behavior | 16:12 |
| danilos | adiroiban, not sure who is it desired by (not by me, even as a user), but it's directly against the spec | 16:13 |
| === gary_poster is now known as gary-lunch | ||
| danilos | adiroiban, I don't want temporary cookies to accumulate in my browser cache | 16:13 |
| adiroiban | danilos: back to our box. I don't like the idea of putting guidelines and details in the same dismissable box, since maybe some users will want to dismiss the guidelines links, but always keep the POTemplate details | 16:13 |
| adeuring | gmb: I think the "elif" in line 58 of the diff is never executed because UNKNOWN apperas in error_message_mapping | 16:14 |
| gmb | adeuring, I know, but all if...elses must have an elif. | 16:15 |
| danilos | adiroiban, when jumping from page to page of the same PO file? I am not convinced, and if so, keeping the single line of guideline links shouldn't be too much of a distraction | 16:15 |
| gmb | adeuring, It protects us from OOPSes down the line if we add an error status but forget to update that property. | 16:15 |
| gmb | adeuring, Hang on, I'm talking nonsense, aren't I... | 16:15 |
| gmb | Wait a minute. | 16:15 |
| adeuring | gmb: ;) | 16:15 |
| adeuring | gmb: argh, no, _I_ am talking nonsense. | 16:16 |
| adiroiban | danilos: no. when going to different templates. the guidelines will keep poping together with the template details. Or are you saying that we should put them in the same box, with different dismiss buttons ? | 16:16 |
| adeuring | mis-read the != operator as == | 16:16 |
| adeuring | ...in this "elif" | 16:17 |
| gmb | adeuring, Ah, yes., | 16:17 |
| adeuring | so, everything is fine | 16:17 |
| gmb | adeuring, I was about to agree with you and remove it. That could have been amusing... | 16:17 |
| danilos | adiroiban, no, it should be a single dismiss button | 16:17 |
| danilos | adiroiban, is that how guidelines dismissal works today? | 16:17 |
| adiroiban | danilos: guideline dismissal state is kept by a sessio cookie. is that the answer for your question ? :) | 16:18 |
| adeuring | gmb: but nevertheless.. I think you can s/elif/else/ and remove the current else part | 16:18 |
| danilos | adiroiban, afaics, they already dismiss guidelines only for the pofile you are on | 16:18 |
| gmb | adeuring, Yes, I agree. | 16:18 |
| danilos | adiroiban, i.e. when you switch templates, you have to dismiss guidelines again | 16:19 |
| danilos | (I keep typing "dismiss" as "dimiss": too many esses in there :) | 16:19 |
| adiroiban | danilos: true. the path is the canonical URL for the pofile | 16:20 |
| adiroiban | danilos: true. the cookie path is the canonical URL for the pofile | 16:20 |
| danilos | adiroiban, I find that just fine, so there should be no issues to use the same box afaict | 16:20 |
| adiroiban | danilos: ok. then we will use the same box | 16:20 |
| danilos | adiroiban, it would be nice to add the collapsing and expanding to it because potemplate details can get big, but that's up to you :) | 16:21 |
| danilos | henninge, what do you think of that? | 16:21 |
| danilos | henninge, i.e. using the existing box for the potemplate description as well? | 16:21 |
| adiroiban | danilos: yes. fold/unfold or (collape/expand) is a must | 16:21 |
| adiroiban | example of long description: https://translations.edge.launchpad.net/ubuntu/lucid/+source/ubiquity-slideshow-ubuntu/+pots/ubiquity-slideshow-ubuntu | 16:22 |
| danilos | adiroiban, I'd probably favour fold/expand or collapse/unfold, but that's just me :P | 16:22 |
| * henninge reads | 16:22 | |
| danilos | adiroiban, yeah, definitely then :) anyway, it'd be nice to add write support for potemplate description so we can have maintainers set it directly in the page :) | 16:23 |
| danilos | API write support, that is | 16:24 |
| adiroiban | danilos: hm... the POTemplates API branch should implment that | 16:24 |
| danilos | adiroiban, then I've done a bad job reviewing it ;) | 16:25 |
| henninge | adiroiban: danilos: They should be seperate boxes. | 16:26 |
| henninge | Remember taht the guildelines are _per language_, therefore per pofile. | 16:27 |
| henninge | whereas the potemplate description is, well, per template ;) | 16:27 |
| henninge | but then again, one translator will usually just work in one language, anyway ... | 16:28 |
| adiroiban | danilos: why ? becouse there are no tests for that? | 16:28 |
| danilos | henninge, yeah, I don't really buy the per-language vs per-template argument :) | 16:28 |
| henninge | danilos: no, I just realized that, too. | 16:29 |
| danilos | adiroiban, could be, and this is something very interesting to make sure we got right, because of the permissions | 16:29 |
| henninge | adiroiban, danilos: let's put them in one popup, expandable and dismissable like it is now. No need for a new cookie then. | 16:30 |
| danilos | henninge, cool, let's do that then :) | 16:30 |
| henninge | but leave it like it is now on the template index page, right? | 16:30 |
| adiroiban | danilos: while having a review for a previous API MP, i was told there is no need to test for security, since they are already tested for the API | 16:30 |
| adiroiban | danilos: since the permissions are checked for the browser, this should also cover the API. Security should be checked only if the security.py mechanism is not used. But maybe I am wrong. | 16:34 |
| adeuring | gmb: r=me. nice work! | 16:43 |
| adiroiban | danilos, henninge: one more insignificat detail: The POTemplate should be indented like in the current PDF, or „Developers notes:” should only be appended to the text in bold and then use the normal paragraph text flow? | 16:46 |
| danilos | adiroiban, right, it should work fine for all the stuff except methods | 16:47 |
| danilos | adiroiban, (on the API permissions) | 16:47 |
| danilos | adiroiban, now, considering it's easy to break permissions without it affecting the UI code, I strongly believe we should have unit tests for these | 16:48 |
| danilos | adiroiban, now, back on the topic: I am not sure, it's best to try it out and see how it works with both approaches | 16:49 |
| danilos | adiroiban, that text should probably not be "Developer's notes" though: we should come up with something more appropriate like "Tips for translating this" | 16:51 |
| danilos | adiroiban, oh, btw, have you considered using something like a help popup for the potemplate details? | 16:51 |
| === deryck[lunch] is now known as deryck | ||
| adiroiban | danilos: yes, but I could not find any strong argument for using a popup | 16:53 |
| adiroiban | it should take the same number of clicks | 16:53 |
| gmb | adeuring, Thanks! | 16:54 |
| adeuring | gmb: welcome. sorry for forgetting an irc notice | 16:54 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | danilos: when adding the „Developer's notes” text, I was keeping in mind the bug 211, and to have both „Notes from other translators” and „Notes from developer” | 16:55 |
| mup | Bug #211: Add comments to POFile for communication between translators <feature> <Launchpad Translations:Triaged> <https://launchpad.net/bugs/211> | 16:56 |
| gmb | adeuring, Can you update the mp so I can ec2 land it? | 16:56 |
| adeuring | gmb: done | 16:56 |
| gmb | adeuring, Thanks | 16:57 |
| danilos | adiroiban, those ancient bugs are a a grab-bag of things and +translate page would need serious redesign to cope with all this :) | 16:57 |
| adiroiban | danilos: to bad. I was thinking that I will be able to solve that bug by using this notification box :( | 16:59 |
| danilos | adiroiban, oohhh, no, pleaaase not | 16:59 |
| danilos | adiroiban, we've got to have a measure of dirty hacks we let in, imo | 17:00 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | yes. but then there will be always something more important than the redesign of +translate page | 17:01 |
| adiroiban | and until the page is redesigned, we can have those dirty hacks/workarounds | 17:01 |
| gmb | adeuring, Can you add a review to that MP? The merge proposal is approved but the code review is still pending. | 17:01 |
| adeuring | gmb: arghh, sorry | 17:02 |
| gmb | adeuring, No worries :) | 17:02 |
| adeuring | gmb: no nmy best day; i had too a few nitpick about the help text but somehow manage to forget click the "save comment" button... | 17:07 |
| adiroiban | danilos: so what will happen with the POTemplate API branch ? are you going to have a new review for it ? | 17:07 |
| danilos | adiroiban, no, I am going to take a quick look to confirm it's all fine and land it as-is; if I don't get it landed I'll let you know | 17:08 |
| adiroiban | danilos: ok. thanks! | 17:09 |
| adiroiban | danilos, henninge: the UX and design for the new +translate page will be done by Rosetta devs or there will be a dedicated UX/Design engineer to handle it? | 17:11 |
| danilos | adiroiban, it'll be done by us mostly, we'll be asking for user input :) as a matter of fact, it's something we can do together before the Epic and then implement it during the Epic :) | 17:12 |
| gmb | adeuring, I see them; I'll fix them. | 17:12 |
| gmb | THanks. | 17:12 |
| adiroiban | danilos: and until now, Henning mockups (I saw them and give feedback for them during the UDS) are the only ideas/design that we have? | 17:14 |
| danilos | adiroiban, yes, they are | 17:14 |
| danilos | adiroiban, btw, where is that feedback? | 17:14 |
| adiroiban | danilos: mrevell should process it. everyhing was audio recorded :) | 17:14 |
| danilos | adiroiban, oh, you took part in the user study, cool :) | 17:15 |
| mrevell | adiroiban, danilos, I'm writing the report right now :) | 17:15 |
| danilos | mrevell, yeah, yeah, as if I believe you :P | 17:15 |
| mrevell | haha, wanna screen shot? :) | 17:16 |
| === salgado is now known as salgado-lunch | ||
| adiroiban | intellectronica: hi. any news regarding the landing of this branch https://code.edge.launchpad.net/~adiroiban/launchpad/bug-570899/+merge/25443 ? :) | 17:21 |
| === gary-lunch is now known as gary_poster | ||
| adiroiban | henninge, danilos: here is what I understood from the previous chat: http://launchpadlibrarian.net/48876861/translate-file-details.pdf :) | 17:30 |
| danilos | adiroiban, looks great :) | 17:30 |
| henninge | adiroiban: really good! | 17:31 |
| bdmurray | Is there a reviewer now? | 17:35 |
| bdmurray | If so I'd like https://code.edge.launchpad.net/~brian-murray/launchpad/bug-546078/+merge/25740 reviewed | 17:37 |
| rockstar | sinzui, can I get a UI review from you? | 17:53 |
| sinzui | yes | 17:53 |
| rockstar | sinzui, https://code.edge.launchpad.net/~rockstar/launchpad/change-recipe-link/+merge/25790 | 17:58 |
| sinzui | rockstar r=me | 18:00 |
| rockstar | sinzui, nice, thanks. | 18:01 |
| * rockstar goes to make the tests pass... | 18:01 | |
| === salgado-lunch is now known as salgado | ||
| deryck | bdmurray, I can't do it right now, but if you haven't found someone before I return later, I can do it then. | 18:09 |
| bdmurray | deryck: okay, thanks. is there a reviewer schedule posted somewhere? | 18:09 |
| deryck | bdmurray, https://dev.launchpad.net/ReviewerSchedule | 18:10 |
| deryck | bdmurray, you can also just poke around the approved reviewers list and ask if anyone has a spare moment. | 18:11 |
| === Ursinha is now known as Ursinha-afk | ||
| === salgado is now known as salgado-afk | ||
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!