Re: Review Request 113173: Reduce warnings noise when including wtf/Platform.h in KJS
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113173/#review41436 --- It makes sense to me. I would like someone else to approve. - David Edmundson On Oct. 8, 2013, 1:20 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113173/ --- (Updated Oct. 8, 2013, 1:20 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Modifies how features are checked. This file has some pre-processor macros that are used to checking for OS features. These are defined with 1 if acceptable and undefined if the feature is not present. These macros are being checked with #if, so the pre-processor will complain whenever it's asked about one of the variables that haven't been defined. This patch adds a defined() first, before checking the value, so the pre-processor doesn't need to consider the variable value, if it's not present. This change makes it possible to read the compilation logs from modules that use KJS. Diffs - tier1/kjs/src/wtf/Platform.h 843cfd2 Diff: http://git.reviewboard.kde.org/r/113173/diff/ Testing --- Builds, KJS tests pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113173: Reduce warnings noise when including wtf/Platform.h in KJS
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113173/#review41458 --- Ship it! Ship It! - Kevin Ottens On Oct. 8, 2013, 1:20 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113173/ --- (Updated Oct. 8, 2013, 1:20 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Modifies how features are checked. This file has some pre-processor macros that are used to checking for OS features. These are defined with 1 if acceptable and undefined if the feature is not present. These macros are being checked with #if, so the pre-processor will complain whenever it's asked about one of the variables that haven't been defined. This patch adds a defined() first, before checking the value, so the pre-processor doesn't need to consider the variable value, if it's not present. This change makes it possible to read the compilation logs from modules that use KJS. Diffs - tier1/kjs/src/wtf/Platform.h 843cfd2 Diff: http://git.reviewboard.kde.org/r/113173/diff/ Testing --- Builds, KJS tests pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113173: Reduce warnings noise when including wtf/Platform.h in KJS
Aleix Pol Gonzalez wrote: This patch adds a defined() first, before checking the value, so the pre-processor doesn't need to consider the variable value, if it's not present. This seems like a bad idea. These are defined with 1 if acceptable and undefined if the feature is not present. This is the problem that should be fixed. It should be 0 if the feature is not present. Thanks, Steve. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113173: Reduce warnings noise when including wtf/Platform.h in KJS
Can we keep all discussions on reviewboard? If you mix replying to the mailing list we lose track of messages very quickly. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113173: Reduce warnings noise when including wtf/Platform.h in KJS
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113173/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- Modifies how features are checked. This file has some pre-processor macros that are used to checking for OS features. These are defined with 1 if acceptable and undefined if the feature is not present. These macros are being checked with #if, so the pre-processor will complain whenever it's asked about one of the variables that haven't been defined. This patch adds a defined() first, before checking the value, so the pre-processor doesn't need to consider the variable value, if it's not present. This change makes it possible to read the compilation logs from modules that use KJS. Diffs - tier1/kjs/src/wtf/Platform.h 843cfd2 Diff: http://git.reviewboard.kde.org/r/113173/diff/ Testing --- Builds, KJS tests pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Reduce warnings noise when including wtf/Platform.h in KJS
Aleix Pol Gonzalez wrote: This patch adds a defined() first, before checking the value, so the pre-processor doesn't need to consider the variable value, if it's not present. This seems like a bad idea. These are defined with 1 if acceptable and undefined if the feature is not present. This is the problem that should be fixed. It should be 0 if the feature is not present. Please fix that instead. Thanks, Steve. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Reduce warnings noise when including wtf/Platform.h in KJS
Stephen Kelly wrote: These are defined with 1 if acceptable and undefined if the feature is not present. This is the problem that should be fixed. It should be 0 if the feature is not present. Please fix that instead. Hmm, or maybe your fix is ok. I don't know where the values come from, but there seems to be a code smell somewhere... Thanks, Steve. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Reduce warnings noise when including wtf/Platform.h in KJS
On Tue, Oct 8, 2013 at 3:50 PM, Stephen Kelly steve...@gmail.com wrote: Stephen Kelly wrote: These are defined with 1 if acceptable and undefined if the feature is not present. This is the problem that should be fixed. It should be 0 if the feature is not present. Please fix that instead. Hmm, or maybe your fix is ok. I don't know where the values come from, but there seems to be a code smell somewhere... Thanks, Steve. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel If you take a look at the code, you'll see that it's what the original author meant. I thought about adding all the alternatives, but I don't think it would make sense, especially given they are meant to be checked through the macros. Aleix ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
