[Qt] use Qt foreach where possible #6227

1. 
   Diapolo commented at 12:51 pm on June 3, 2015: none

   • also use const references where appropriate
2. 
   jonasschnelli commented at 12:58 pm on June 3, 2015: contributor

   Codereview utACK. Next time it would recommend to split of the const changes and brackets/line break changes into a different commit.
3. 
   Diapolo commented at 1:05 pm on June 3, 2015: none

   @jonasschnelli I just wanted to ensure I’m using the correct style that get’s applied after clang-cleanup and changed it in the same commit. I hope this is still okay for this pull :). Thanks for review.
4. 
   laanwj commented at 3:06 pm on June 3, 2015: member

   Not opposed, although I’d prefer to go straight to c++11 range-based for if we’re going that route anyway - that avoids some diff noise.
5. laanwj added the label Improvement on Jun 3, 2015
6. [Qt] use Qt foreach where possible

   - also use const references where appropriate

   1c10b09941
7. 
   Diapolo commented at 9:50 am on June 12, 2015: none

   @laanwj Yeah, a move to C++11 would be nice, but until there is consensus about when and how, we could perhaps merge this rather uncontroversial pull.
8. 
   laanwj commented at 2:45 pm on June 12, 2015: member

   I really want to reach consensus on C++11. I don’t see the need for this intermediate step, it’s just replacing one macro with another.
9. 
   laanwj commented at 5:41 pm on July 2, 2015: member

   Closing this, please don’t submit any pulls that don’t fix any issues anymore, we don’t have time nor resources to review them.
10. laanwj closed this on Jul 2, 2015

    ---

11. 
    Diapolo commented at 5:20 am on July 3, 2015: none

    @laanwj May I remember you that you were it, who tagged this “improvement”. Also it’s not my fault that time of the core devs is limited. It’s also my free time, I use to create patches for Bitcoin Core and it’s not very motivating at all to just hear that same phrase again. You even reviewed this and @jonasschnelli gave an ACK… why is the discussion always longer than just merging it?
12. 
    jonasschnelli commented at 5:44 am on July 3, 2015: contributor

    I’m with @laanwj on this: let’s wait and change it directly to a c++11.
13. 
    luke-jr commented at 5:52 am on July 3, 2015: member

    OTOH, if we’re leaning toward not moving to C++11 for years (what it now looks like the outcome may be), might as well merge this…
14. 
    laanwj commented at 7:25 am on July 3, 2015: member

    @diapolo But why? This patch introduces an ever-small amount of risk at no gain at all. Boost’s foreach works just as well as Qt’s, and at least the code is verified to work that way.

    I understand it is your free time too, and I’m sorry for that, but that is no argument to merge everything. Mind that this is a highly critical piece of software. We can’t just merge things for the sake of being a lot of work.

    it’s not very motivating at all to just hear that same phrase again

    I’m only repeating that because you don’t pay attention to it. Please try to contribute changes that fix reported issues, not just ‘because I think the code is prettier this way’.

15. Diapolo deleted the branch on Jul 6, 2015
16. DrahtBot locked this on Sep 8, 2021

Contributors
Diapolo jonasschnelli laanwj luke-jr

Labels
Refactoring