Re: Review Request 118947: KJS: treat specified time correctly as localtime when passed to the Date() constructor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/#review61013 --- Ship it! Good catch! Bug by me, I passed the correct utc value to makeTimeFromList but never passed it to makeTime (where it should go), so the fix is 100% correct. - Bernd Buschinski On June 26, 2014, 10:41 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/ --- (Updated June 26, 2014, 10:41 a.m.) Review request for kdelibs. Bugs: 335556 http://bugs.kde.org/show_bug.cgi?id=335556 Repository: kdelibs Description --- The commit https://projects.kde.org/projects/kde/kdelibs/repository/revisions/48dd1fc50277b861b49613c5f46b6f8b10cc932d introduced a bug: It factors out the code to convert a time specification to an actual time value into the function makeTimeFromList(). But in that function makeTime(t, ms, true) is called, whereas the original code called makeTime(t, ms, false) for the constructor. This patch fixes it by passing the utc parameter to makeTime() instead of true. Please note that this bug is also present in Frameworks5. Diffs - kjs/date_object.cpp c8d776c Diff: https://git.reviewboard.kde.org/r/118947/diff/ Testing --- Loaded the test case from the bug report into Konqueror/KHTML. The correct time is shown now like in other browsers, including Konqueror/WebKit. Thanks, Wolfgang Bauer
Re: Review Request 118763: Remove find_package(XCB) call as it is handled already by the top-level cmake file
On June 16, 2014, 6:57 a.m., Martin Gräßlin wrote:
looks good to me, +1. Please add Hugo Pereira Da Costa to the Review
Request, though.
The review request made me wonder whether we still need to find XLib in
Oxygen, though. The parts shown only use XCB, so maybe we could just go for
finding only XCB?
@Martin,
yes you are probably right. X11 should not be necessary any more.
I'll double check and commit. (have other stuff pending).
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/#review60162
---
On June 16, 2014, 2:07 p.m., Bernd Steinhauser wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/
---
(Updated June 16, 2014, 2:07 p.m.)
Review request for kde-workspace, Plasma and Hugo Pereira Da Costa.
Repository: oxygen
Description
---
No idea if kde-workspace is still the right group, if not, please change.
find_package(XCB) is called without specifying the required components. This
leads to linking to unused dependencies in case they are installed.
Since XCB is searched for in the top level cmake file in the repository,
there is no need to search for it again. The component required there (only
base XCB) is sufficient.
Although, this should be sufficient to fix the deps problem, it makes sense
to link to XCB::XCB instead of ${XCB_LIBRARIES}, since the former is what is
actually needed.
Diffs
-
kstyle/CMakeLists.txt 165b62a
liboxygen/CMakeLists.txt 0d1dd94
Diff: https://git.reviewboard.kde.org/r/118763/diff/
Testing
---
Thanks,
Bernd Steinhauser
Re: Review Request 118763: Remove find_package(XCB) call as it is handled already by the top-level cmake file
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/#review61017
---
Ship it!
Ship It!
- Hugo Pereira Da Costa
On June 16, 2014, 2:07 p.m., Bernd Steinhauser wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/
---
(Updated June 16, 2014, 2:07 p.m.)
Review request for kde-workspace, Plasma and Hugo Pereira Da Costa.
Repository: oxygen
Description
---
No idea if kde-workspace is still the right group, if not, please change.
find_package(XCB) is called without specifying the required components. This
leads to linking to unused dependencies in case they are installed.
Since XCB is searched for in the top level cmake file in the repository,
there is no need to search for it again. The component required there (only
base XCB) is sufficient.
Although, this should be sufficient to fix the deps problem, it makes sense
to link to XCB::XCB instead of ${XCB_LIBRARIES}, since the former is what is
actually needed.
Diffs
-
kstyle/CMakeLists.txt 165b62a
liboxygen/CMakeLists.txt 0d1dd94
Diff: https://git.reviewboard.kde.org/r/118763/diff/
Testing
---
Thanks,
Bernd Steinhauser
Re: Review Request 118947: KJS: treat specified time correctly as localtime when passed to the Date() constructor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/#review61018 --- This review has been submitted with commit 0137c691cfe02e6fc0c99e2ced15aea149e56b4b by Wolfgang Bauer to branch KDE/4.13. - Commit Hook On June 26, 2014, 10:41 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/ --- (Updated June 26, 2014, 10:41 a.m.) Review request for kdelibs. Bugs: 335556 http://bugs.kde.org/show_bug.cgi?id=335556 Repository: kdelibs Description --- The commit https://projects.kde.org/projects/kde/kdelibs/repository/revisions/48dd1fc50277b861b49613c5f46b6f8b10cc932d introduced a bug: It factors out the code to convert a time specification to an actual time value into the function makeTimeFromList(). But in that function makeTime(t, ms, true) is called, whereas the original code called makeTime(t, ms, false) for the constructor. This patch fixes it by passing the utc parameter to makeTime() instead of true. Please note that this bug is also present in Frameworks5. Diffs - kjs/date_object.cpp c8d776c Diff: https://git.reviewboard.kde.org/r/118947/diff/ Testing --- Loaded the test case from the bug report into Konqueror/KHTML. The correct time is shown now like in other browsers, including Konqueror/WebKit. Thanks, Wolfgang Bauer
Re: Review Request 118947: KJS: treat specified time correctly as localtime when passed to the Date() constructor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/ --- (Updated June 26, 2014, 12:03 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Bugs: 335556 http://bugs.kde.org/show_bug.cgi?id=335556 Repository: kdelibs Description --- The commit https://projects.kde.org/projects/kde/kdelibs/repository/revisions/48dd1fc50277b861b49613c5f46b6f8b10cc932d introduced a bug: It factors out the code to convert a time specification to an actual time value into the function makeTimeFromList(). But in that function makeTime(t, ms, true) is called, whereas the original code called makeTime(t, ms, false) for the constructor. This patch fixes it by passing the utc parameter to makeTime() instead of true. Please note that this bug is also present in Frameworks5. Diffs - kjs/date_object.cpp c8d776c Diff: https://git.reviewboard.kde.org/r/118947/diff/ Testing --- Loaded the test case from the bug report into Konqueror/KHTML. The correct time is shown now like in other browsers, including Konqueror/WebKit. Thanks, Wolfgang Bauer
Review Request 118954: Set directory to current working directory when executing Open With... dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118954/ --- Review request for kdelibs and David Faure. Bugs: 142597 http://bugs.kde.org/show_bug.cgi?id=142597 Repository: kdelibs Description --- The attached patch sets the current directory to the current working directory (read: directory in which the open with dialog was run) whenever the KService path is empty. That way we tell KProcess in the context of the current directory instead of the default one it uses, $HOME. Diffs - kio/kio/krun.cpp 590fcf8 Diff: https://git.reviewboard.kde.org/r/118954/diff/ Testing --- Tested with the example provided in the bug report. Verified the output of the compile process is in the current working directory instead of $HOME. Thanks, Dawit Alemayehu
Re: Review Request 118947: KJS: treat specified time correctly as localtime when passed to the Date() constructor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/#review61019 --- This review has been submitted with commit 786e0a7821f195c5a227e72a41d29033851dc2af by Wolfgang Bauer to branch master. - Commit Hook On June 26, 2014, 12:03 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/ --- (Updated June 26, 2014, 12:03 p.m.) Review request for kdelibs. Bugs: 335556 http://bugs.kde.org/show_bug.cgi?id=335556 Repository: kdelibs Description --- The commit https://projects.kde.org/projects/kde/kdelibs/repository/revisions/48dd1fc50277b861b49613c5f46b6f8b10cc932d introduced a bug: It factors out the code to convert a time specification to an actual time value into the function makeTimeFromList(). But in that function makeTime(t, ms, true) is called, whereas the original code called makeTime(t, ms, false) for the constructor. This patch fixes it by passing the utc parameter to makeTime() instead of true. Please note that this bug is also present in Frameworks5. Diffs - kjs/date_object.cpp c8d776c Diff: https://git.reviewboard.kde.org/r/118947/diff/ Testing --- Loaded the test case from the bug report into Konqueror/KHTML. The correct time is shown now like in other browsers, including Konqueror/WebKit. Thanks, Wolfgang Bauer
Re: Review Request 118947: KJS: treat specified time correctly as localtime when passed to the Date() constructor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/#review61020 --- This review has been submitted with commit 198c1819e382aed93a574c6f9d51c7303985a672 by Wolfgang Bauer to branch master. - Commit Hook On June 26, 2014, 12:03 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/ --- (Updated June 26, 2014, 12:03 p.m.) Review request for kdelibs. Bugs: 335556 http://bugs.kde.org/show_bug.cgi?id=335556 Repository: kdelibs Description --- The commit https://projects.kde.org/projects/kde/kdelibs/repository/revisions/48dd1fc50277b861b49613c5f46b6f8b10cc932d introduced a bug: It factors out the code to convert a time specification to an actual time value into the function makeTimeFromList(). But in that function makeTime(t, ms, true) is called, whereas the original code called makeTime(t, ms, false) for the constructor. This patch fixes it by passing the utc parameter to makeTime() instead of true. Please note that this bug is also present in Frameworks5. Diffs - kjs/date_object.cpp c8d776c Diff: https://git.reviewboard.kde.org/r/118947/diff/ Testing --- Loaded the test case from the bug report into Konqueror/KHTML. The correct time is shown now like in other browsers, including Konqueror/WebKit. Thanks, Wolfgang Bauer
Re: Bug 92237: patch submitted, but... is anyone watching?
On 24.06.2014 11:10, Frank Reininghaus wrote: Hi, 2014-06-22 13:04 GMT+02:00 Simon Bachmann: May I draw your attention to bug 92237? (https://bugs.kde.org/show_bug.cgi?id=92237) No, this is not a please-get-this-issue-fixed-ASAP kind of message. It's just that I submitted a patch to that bug a few weeks ago, and I'm afraid no one noticed I see two comments which were submitted after your patch. You were asked to submit the patch via http://git.reviewboard.kde.org/ and to answer some questions concerning your patch, so it seems that it was definitely noticed ;-) Regards, Frank Ops... somehow i didn't get the e-mail notification that comments were added, and I didn't check the bugtracker before posting here. So, sorry for the noise! I'll submit the patch to the reviewboard one of these days. Regards, Simon
