This revision was automatically updated to reflect the committed changes.
Closed by commit R246:bf8f3067fd3a: Loader: Avoid Q_FOREACH (authored by
loh.tar, committed by cullmann).
REPOSITORY
R246 Sonnet
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17028?vs=45838=47185
REVISION
cullmann added a comment.
We started to accept his patches for other frameworks, I think there is no
harm in doing so, here, too. Real name or not.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
To: loh.tar, davidedmundson
Cc: cullmann, cfeck, smartins,
loh.tar added a comment.
>> @cfeck wrote:
>
> It looked strange on the list of names of all committers.
A look at your page indicates that you are doing some public relations?
Should you copy some data from an auto generated list, feel free to
remove me.
REPOSITORY
R246 Sonnet
loh.tar added a comment.
> ..but I cannot remember we didn't ask contributors for their real name
for contributions.
Um, what? I was asked. I hope it's ok as it is.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
To: loh.tar, davidedmundson
Cc: cfeck,
cfeck added a comment.
I do not know if there is a written rule that we require commits with real
names (for legal reasons), but I cannot remember we didn't ask contributors for
their real name for contributions.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
loh.tar added a comment.
Well, neither of that. My "online me" is loh.tar but that was rejected by
Phabricator, so the dot had to go. Lastly used ngraham uppercase letters, what
looks even more strange to me :-)
Should you intend to commit this, thanks in advance! May you look at D17055
cfeck added a comment.
Is your full name really "loh tar", spelled with lower case letters? It
looked strange on the list of names of all committers.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
To: loh.tar, davidedmundson
Cc: cfeck, smartins,
loh.tar added a comment.
@davidedmundson
May I ask how does this here will progress? You seemed to request a change
which was not clear to me, but it's anyway "green flagged".
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
To: loh.tar, davidedmundson
Cc:
loh.tar added inline comments.
INLINE COMMENTS
> davidedmundson wrote in loader.cpp:273
> > Probably, yes. But Qt docu always says, "thanks to implicit sharing copying
> > a container is very fast"
>
> That's mixing up docs.
>
> foreach() always does a const copy of the list. This is a cheap
davidedmundson added inline comments.
INLINE COMMENTS
> loh.tar wrote in loader.cpp:273
> Probably, yes. But Qt docu always says, "thanks to implicit sharing copying a
> container is very fast"
>
> Is copy previous into a const var a benefit (as I have seen recently)? Don't
> think so. Or
loh.tar added inline comments.
INLINE COMMENTS
> smartins wrote in loader.cpp:273
> will languages() detach ?
Probably, yes. But Qt docu always says, "thanks to implicit sharing copying a
container is very fast"
Is copy previous into a const var a benefit (as I have seen recently)? Don't
davidedmundson added a comment.
> .I have now done a search for more Q_FOREACH/foreach. Please let me know if
you want each as own diff or all in once
Can be all at once, but still go via review. There are some foreach / for
differences and pitfalls to be wary of.
REPOSITORY
R246
smartins added inline comments.
INLINE COMMENTS
> loader.cpp:273
> QStringList allLocalizedDictionaries;
> -Q_FOREACH (const QString , languages()) {
> +for (const QString : languages()) {
> allLocalizedDictionaries.append(languageNameForCode(langCode));
will languages()
loh.tar added a comment.
@davidedmundson Wow, that was fast :-)
I have now done a search for more Q_FOREACH/foreach. Please let me know if
you want each as own diff or all in once.
data/parsetrigrams.cpp
src/core/guesslanguage.cpp
src/plugins/hspell/hspelldict.cpp
davidedmundson accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
To: loh.tar, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
loh.tar created this revision.
Herald added a project: Frameworks.
loh.tar requested review of this revision.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D17028
AFFECTED FILES
src/core/loader.cpp
To: loh.tar
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
16 matches
Mail list logo
