Re: Review Request: parenthesis and not null checking
On Nov. 10, 2012, 10:21 p.m., Rolf Eike Beer wrote:
A simple way to verify if this is correct is check if you and the compiler
agree on the code. Run make -n in the build tree to get the full compiler
command line, then insert a -S (assuming you are using gcc) and change
the -o to point to a temporary file. This will output the assembler code.
Do this with and without your modifications and look if the result is still
the same.
yes, I've done it. They match.
bool kk1()
{
int a = 1;
int b = 2;
return (!a ^ b);
}
bool kk2()
{
int a = 1;
int b = 2;
return (!(a ^ b));
}
bool kk3()
{
int a = 1;
int b = 2;
return (!(a) ^ b);
}
kk1 and kk3 produce the same assembler output.
But the problem is if the author really wanted the parenthesis that way or not.
for example, (!a) ^ b (what gcc produces) vs. !(a ^ b) (what the author
wanted?)
- Jaime Torres
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107213/#review21785
---
On Nov. 5, 2012, 3:13 p.m., Jaime Torres Amate wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107213/
---
(Updated Nov. 5, 2012, 3:13 p.m.)
Review request for kdelibs.
Description
---
place some parenthesis around ! operators, with less priority than ^
operators.
place some parenthesis around = inside conditions
check for n not being null before using it
simplify if (a==true) return true else return false;
Diffs
-
khtml/khtml_caret.cpp 45fd90c
khtml/rendering/render_inline.cpp acfc1e4
khtml/rendering/render_object.cpp f37627c
solid/solid/backends/wmi/wmiopticaldisc.cpp c6e390f
Diff: http://git.reviewboard.kde.org/r/107213/diff/
Testing
---
I've been using this code several weeks.
Thanks,
Jaime Torres Amate
kde-workspace/4.9 messed up
Hi, somehow the master branch of kde-workspace has been merged in the KDE/4.9 branch, with obvious consequences. Can anyone please fix this mess? Thanks, -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: Review Request: Make sure ksmserver ignores excluded apps when saving session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/#review21816 --- ksmserver/server.cpp http://git.reviewboard.kde.org/r/107276/#comment16844 why not check for both program and filename? that should then catch 1, 2 and 4, no? excludeApps could also be pre-processed to include both long paths and filenames which would then allow catching all 4 variations. probably the reason this was written to only catch 1 and 4, however, was in case there were binaries of the same name in different paths that should be treated differently (allowing differentiation by full path).. Lubos will certainly have more insight on this, however. - Aaron J. Seigo On Nov. 10, 2012, 12:45 p.m., Jekyll Wu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- (Updated Nov. 10, 2012, 12:45 p.m.) Review request for kdelibs, Plasma and Luboš Luňák. Description --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), this patch works with 1) and 2) . I don't know whether it make senses to support all combinations This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs - ksmserver/server.cpp eb3ac18 Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Re: What is the git workflow for kdelibs ?
On Saturday 10 November 2012 Alexander Neundorf wrote: community.kde.org feels to me like a completely unorganized heap of random stuff. That's true to some degree, and it's exactly the reason why we crated community.kde.org, because we don't want to put this unorganized stuff, which is only understandable, if you have inside knowledge, to 3rd parties as documentation. Techbase is supposed this organized documentation understandable by 3rd parties, so only the stuff, which fits this should go there. -- Cornelius Schumacher schumac...@kde.org
Re: kde-workspace/4.9 messed up
On Sunday 11 November 2012 12:00:05 Pino Toscano wrote: Hi, somehow the master branch of kde-workspace has been merged in the KDE/4.9 branch, with obvious consequences. Can anyone please fix this mess? Thanks, Uh oh, I think I did git reset origin/HEAD --hard in the 4.9 branch, to get rid of an initial commit I made in that branch. So HEAD doesn't refer to the remote head in the current branch, but always to the head of master? Christoph Feck (kdepepo)
Re: kde-workspace/4.9 messed up
On domingo, 11 de novembro de 2012 12.48.15, Christoph Feck wrote: Uh oh, I think I did git reset origin/HEAD --hard in the 4.9 branch, to get rid of an initial commit I made in that branch. So HEAD doesn't refer to the remote head in the current branch, but always to the head of master? HEAD is a branch. It's not symbolic like it was in SVN or CVS (the tip of a given branch). By default, HEAD is actually a symbolic ref -- it points to another ref. $ cat .git/refs/remotes/origin/HEAD ref: refs/remotes/origin/master So you see exactly what origin/HEAD is. You can also change it (locally) using git remote set-head. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: kde-workspace/4.9 messed up
Christoph Feck wrote: On Sunday 11 November 2012 12:00:05 Pino Toscano wrote: Hi, somehow the master branch of kde-workspace has been merged in the KDE/4.9 branch, with obvious consequences. Can anyone please fix this mess? Thanks, Uh oh, I think I did git reset origin/HEAD --hard in the 4.9 branch, to get rid of an initial commit I made in that branch. So HEAD doesn't refer to the remote head in the current branch, but always to the head of master? Something like that. It's similar to the mess that was just fixed yesterday in kdelibs: http://thread.gmane.org/gmane.comp.kde.devel.core/76984 Amazing that two people make the same mistake in such a short time, after such a long time without a similar issue. Someone needs to force-push the branch. Sysadmin knows what to do, so please talk to them to get it fixed. Thanks, Steve.
Review Request: Make sure ksmserver ignores excluded apps when saving session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- Review request for kdelibs, Plasma and Luboš Luňák. Description --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), this patch works with 1) and 2) . I don't know whether it make senses to support all combinations This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs - Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request: Make sure ksmserver ignores excluded apps when saving session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- (Updated Nov. 10, 2012, 12:45 p.m.) Review request for kdelibs, Plasma and Luboš Luňák. Changes --- Forgot uploading the patch Description --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), this patch works with 1) and 2) . I don't know whether it make senses to support all combinations This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs (updated) - ksmserver/server.cpp eb3ac18 Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Re: kde-workspace/4.9 messed up
Hi all, This has now been corrected - KDE/4.9 has been rewound to f7fb0c10fa6bc04082963fa5863c579d560dc4a1. I have also blacklisted one of the commit hashes so this cannot re-enter the repository. You need to run git reset --hard origin/KDE/4.9 with the KDE/4.9 branch checked out to reset yourself to a state where you will be able to push again if you work on the KDE/4.9 branch. (As an aside, local HEAD always points to the branch you have checked out - origin/HEAD points to the default checkout branch of the remote repository) Regards, Ben Cooksley KDE Sysadmin
