Formatting/gramattical fixes in fees.h #7268

pull hoffmabc wants to merge 1 commits into bitcoin:master from hoffmabc:fix-typos-fees changing 1 files +17 −17

hoffmabc commented at 3:05 PM on December 31, 2015: none

Extremely minor fixes for spacing, periods, extra words.
luke-jr commented at 3:11 PM on December 31, 2015: member

ACK (although not sure we even want double-spaces...)
hoffmabc commented at 3:12 PM on December 31, 2015: none

Well I believe it was triple-spaced :) so just being consistent.
dcousens commented at 2:33 PM on January 1, 2016: contributor

utACK
jonasschnelli commented at 7:23 AM on January 2, 2016: contributor

ACK c3dacfd7f2ad4812b223ec6adca1578d6c030fe1
fanquake commented at 7:55 AM on January 2, 2016: member

There are a few more changes that can be made: s/calcuate/calculate on lines 90 and 96 spaces at the ends of line 140 and 143

ACK pending the above nits
in src/policy/fees.h:None in c3dacfd7f2 outdated
```
 224 | @@ -225,19 +225,19 @@ class CBlockPolicyEstimator
 225 |      void processBlock(unsigned int nBlockHeight,
 226 |                        std::vector<CTxMemPoolEntry>& entries, bool fCurrentEstimate);
 227 |  
 228 | -    /** Process a transaction confirmed in a block*/
 229 | +    /** Process a transaction confirmed in a block */
```
MarcoFalke commented at 12:47 PM on January 2, 2016:

NACK on those. Doxygen doesn't care about such white space. IIrc we already had this discussion.

hoffmabc commented at 2:31 PM on January 2, 2016:

If we added the CommentPragmas spacing then this would be taken care of as well I believe.

MarcoFalke commented at 3:29 PM on January 2, 2016:

I don't think CommentPragmas support this. They are a regular expression that describes comments with special meaning, which should not be split into lines or otherwise changed.

MarcoFalke commented at 3:31 PM on January 2, 2016:

Again, doxygen doesn't care about such white space and I don't think this diff is required.
in src/policy/fees.h:None in c3dacfd7f2 outdated
```
  78 | @@ -79,7 +79,7 @@ class CTxMemPool;
  79 |  class TxConfirmStats
  80 |  {
  81 |  private:
  82 | -    //Define the buckets we will group transactions into (both fee buckets and priority buckets)
  83 | +    // Define the buckets we will group transactions into (both fee buckets and priority buckets)
```
MarcoFalke commented at 12:57 PM on January 2, 2016:

If we really want this, shouldn't clang-format take care of those?

hoffmabc commented at 2:30 PM on January 2, 2016:

Do you want to add a CommentPragmas to enforce the space comprehensively?

MarcoFalke commented at 3:30 PM on January 2, 2016:

<strike>Looks like cf is missing this feature...

We'd need to set ColumnLimit to not-null, to support this.
morcos commented at 2:52 PM on January 2, 2016: member

I wrote this code, so apologies on the mistakes and ACK on the changes.

But in general I'm not sure its worth the review time for minor comment fixes like this, especially in relatively new code. I anticipate that this code will be changed somewhat significantly over the next few releases. There is much other code that needs commenting in the first place or editing for clarity of understanding.

Formatting/gramattical fixes

Extremely minor fixes for spacing, periods, extra words.

Spacing and spelling mistake

9fef8f633d

hoffmabc force-pushed on Jan 2, 2016
hoffmabc commented at 2:57 PM on January 2, 2016: none

@fanquake resolved those nits you mentioned.
paveljanik commented at 1:05 PM on January 3, 2016: contributor

@hoffmabc Please add dots at the end of "We have to lump..." paragraph. And also fix your typo in the commit description (gramattical). I like one space too.
laanwj commented at 9:54 AM on January 16, 2016: member

Closing this, it creates too much discussion for a simple whitespace change.

Also agree with @morcos . This code is still heavily in flux - more detailed documentation is always welcome, but it's not quite time to dot the final i's yet.
laanwj closed this on Jan 16, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors