| === bpierre_ is now known as bpierre | ||
| === [0xAA] is now known as Zer0Pings | ||
| === davmor2_ is now known as davmor2 | ||
| kalikiana | timp: https://code.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-toolkit/contextPropertyWindowQStringLiteral/+merge/307154 | 10:34 |
|---|---|---|
| timp | kalikiana: why did I never see that warning? | 10:36 |
| timp | what is the warning? | 10:36 |
| kalikiana | timp: See the description. Warnings are still flaky | 10:36 |
| kalikiana | Have been for a long time | 10:37 |
| kalikiana | Unless one were to use clang | 10:37 |
| timp | yeah, I know. But I never saw a warning when testing locally (on yakkety) | 10:37 |
| timp | ah the warning only shows with clang? | 10:37 |
| kalikiana | timp: I just replied to that :-) | 10:37 |
| kalikiana | No | 10:37 |
| kalikiana | I'm saying clang is 100% consistent, gcc is random | 10:37 |
| timp | oh... nice ;) | 10:37 |
| timp | kalikiana: ok, happroved. | 10:38 |
| kalikiana | I've complained about it before... and I was thinking loicm_'s work was meant to fix that, but it hasn't evidently | 10:38 |
| kalikiana | (But I stopped using clang because I don't constantly want to fix build errors) | 10:38 |
| kalikiana | (Thanks) | 10:41 |
| loicm_ | kalikiana: was it generating warnings on all builds or on specific Qt (or Ubuntu) version? | 10:44 |
| kalikiana | loicm_: I've seen it with Qt 5.6 and Xenial | 10:45 |
| kalikiana | The build actually continued | 10:45 |
| loicm_ | kalikiana: the warnings were fixed for Qt 5.5.1, not sure we were testing against 5.6 on CI yet | 10:49 |
| kalikiana | loicm_: Even if we weren't then, it should be failing the build now... | 10:58 |
| kalikiana | But it does not | 10:58 |
| loicm_ | kalikiana: right, but that's another issue | 10:59 |
| loicm_ | kalikiana: is it a custom compiled Qt? | 10:59 |
| kalikiana | No, it's what comes with Xenial | 11:00 |
| kalikiana | It's not "another issue" exactly - if it had failed the build, CI would've rejected the MR :-) | 11:00 |
| kalikiana | Usually I rely on that | 11:01 |
| timp | I added this contextproperty call in parallel with loicm_ adding all the QStringLiterals, that's why it was missing for this instance. | 11:02 |
| loicm_ | kalikiana: it's linked obviously but fixing the QStringLiteral issue won't fix the warnings ar not errors on Qt 5.6, so two different issues with two different fixes required | 11:08 |
| loicm_ | timp: ah ok | 11:08 |
| loicm_ | kalikiana: I'll have to check what happens here, do you still have the link to the CI errors? | 11:08 |
| loicm_ | kalikiana: to check the flags | 11:09 |
| kalikiana | loicm_: You can see it here https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1300/consoleText - search for setContextProperty | 11:22 |
| kalikiana | loicm_: http://paste.ubuntu.com/23250707/ | 11:23 |
| === _salem is now known as salem_ | ||
| loicm_ | kalikiana (timp, zsombi): the reason why the implicit char*->QString conversion emitted a warning but didn't generated an error even though we have -Werror is because we also have -Wno-error=deprecated-declarations, all that is handled on purpose by the qmake warnings_are_errors option | 12:36 |
| loicm_ | they reason they explicitly add -Wno-error=deprectaed-declarations is explained here https://codereview.qt-project.org/#/c/63414/ | 12:37 |
| zsombi | loicm_: oh, so this means that if we take that away, we'd get rest of the string failures on Linux too... | 12:37 |
| loicm_ | zsombi: I guess so, but it depends on the compiler | 12:38 |
| loicm_ | kalikiana, zsombi: we don't have the same issue regarding -Wno-error=deprecated-declarations since we don't ship tarballs for our releases (AFAIK), but that's definitely a good reason | 12:41 |
| kalikiana | loicm_: Hrm yeah, I can see the point. From my/ our point of view I'd say it's a little different in that specifically for CI all warnings can expected to be fixed - if that makes sense we could perhaps override it in the debian/rules specifically, so anyone using branches elsewhere won't have to deal with new warnings | 12:48 |
| === chihchun is now known as chihchun_afk | ||
| loicm_ | kalikiana: that's also what I think would be the best, it could be done by removing that flag when compiled with the debian_build option (passed at qmake time in the debian rules) | 12:56 |
| loicm_ | kalikiana: but that also means that by default when developing, these warnings would go unnoticed and appear only in CI, which could be a little frustrating | 12:59 |
| loicm_ | kalikiana: I don't think that's too problematic though since we'll switch on the silent option and make these warnings much more visible | 12:59 |
| loicm_ | kalikiana: together with the colored GCC messages that are one by default now | 13:00 |
| kalikiana | Generally speaking I'd argue that anyway you are going to run into errors that only occur on CI | 13:00 |
| loicm_ | kalikiana: yup | 13:01 |
| loicm_ | I'm adding that to my TODO then | 13:01 |
| kalikiana | Ideally we could enable warnings in qmake based on the systems we have CI for - but I'm not sure how feasible that is | 13:01 |
| kalikiana | loicm_: You're saying we'll have silent flags in CI? I always have them locally because I'd go mad otherwise, but I thought there's a reason we don't do it out of the box | 13:02 |
| loicm_ | kalikiana: I think silent should be on only when !debian_build | 13:03 |
| loicm_ | kalikiana: because on CI you want to see the flags | 13:03 |
| kalikiana | Hmmkay I guess in that case it's not relevant to me | 13:03 |
| === JanC is now known as Guest12605 | ||
| === JanC_ is now known as JanC | ||
| === [0xAA] is now known as Zer0Pings | ||
| === chihchun_afk is now known as chihchun | ||
| === diddledan_ is now known as diddledan | ||
| === salem_ is now known as _salem | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!