Clarify include guard naming convention #12757

practicalswift commented at 3:30 PM on March 22, 2018: contributor

Documentation: Document include guard convention
Fix: Fix missing or inconsistent include guards
Regression test: Add lint-include-guards.sh which checks include guard consistency

Document include guard convention 8af65d96f4

Fix missing or inconsistent include guards 8fd6af89a0

practicalswift force-pushed on Mar 22, 2018

promag commented at 6:00 PM on March 22, 2018: member

Concept ACK.

Could run script in travis where CHECK_DOC=1 to prevent future PR's breaking it?

practicalswift commented at 6:05 PM on March 22, 2018: contributor

@promag contrib/devtools/lint-*.sh is executed automatically by Travis (via lint-all.sh which is run as part of CHECK_DOC=1). In other words contrib/devtools/lint-include-guards.sh will be executed by Travis – no need to reference it explicitly :-)

in contrib/devtools/lint-include-guards.sh:6 in ff981df38c outdated

   0 | @@ -0,0 +1,24 @@
   1 | +#!/bin/bash
   2 | +
   3 | +HEADER_ID_PREFIX="BITCOIN_"
   4 | +HEADER_ID_SUFFIX="_H"
   5 | +
   6 | +REGEXP_EXCLUDE_FILES_WITH_PREFIX="src/(config/bitcoin-config.h|crypto/ctaes/|leveldb/|obj/build.h|qt/forms/|secp256k1/|tinyformat.h|univalue/)"

eklitzke commented at 6:17 PM on March 22, 2018:

Do you really need bitcoin-config.h and build.h in here? They're auto-generated so I would have thought git ls-files wouldn't have listed them.

in contrib/devtools/lint-include-guards.sh:1 in ff981df38c outdated

   0 | @@ -0,0 +1,24 @@
   1 | +#!/bin/bash

eklitzke commented at 6:18 PM on March 22, 2018:

You should probably add a copyright notice to this file.

eklitzke commented at 6:18 PM on March 22, 2018: contributor

Concept ACK

practicalswift force-pushed on Mar 22, 2018

Add lint-include-guards.sh which checks include guard consistency 3bcc0059b8

practicalswift force-pushed on Mar 22, 2018

practicalswift commented at 6:42 PM on March 22, 2018: contributor

@eklitzke Thanks for reviewing! Feedback addressed. Please re-review :-)

sipa commented at 9:13 PM on March 22, 2018: member

utACK 3bcc0059b82b1d482d499d4d8f09118617074a5b

MarcoFalke closed this on Mar 22, 2018

MarcoFalke reopened this on Mar 22, 2018

in contrib/devtools/lint-include-guards.sh:15 in 3bcc0059b8

  10 | +HEADER_ID_SUFFIX="_H"
  11 | +
  12 | +REGEXP_EXCLUDE_FILES_WITH_PREFIX="src/(crypto/ctaes/|leveldb/|secp256k1/|tinyformat.h|univalue/)"
  13 | +
  14 | +EXIT_CODE=0
  15 | +for HEADER_FILE in $(git ls-files -- "*.h" | grep -vE "^${REGEXP_EXCLUDE_FILES_WITH_PREFIX}")

promag commented at 11:05 PM on March 22, 2018:

Could use git ls-files -x instead of | grep -vE?

promag commented at 11:10 PM on March 22, 2018:

Never mind, -x skips untracked files.

practicalswift commented at 11:12 PM on March 22, 2018:

@promag No I don't think so (-x is for untracked files, right?) :-)

I'd be glad to be proven wrong so provide an equivalent using git ls-files -x and I'll switch :-)

promag commented at 11:14 PM on March 22, 2018:

Yes I've come to that conclusion right after trying it.

promag commented at 11:13 PM on March 22, 2018: member

Tested ACK 3bcc005 on mac.

eklitzke approved

eklitzke commented at 7:44 AM on March 27, 2018: contributor

Thanks! utACK 3bcc0059b82b1d482d499d4d8f09118617074a5b

MarcoFalke merged this on Apr 1, 2018

MarcoFalke closed this on Apr 1, 2018

MarcoFalke referenced this in commit 0c5f67b8e5 on Apr 1, 2018

str4d referenced this in commit 768cd14cf9 on May 4, 2018

zkbot referenced this in commit 19ac942045 on May 7, 2018

zkbot referenced this in commit 42037a9b35 on May 7, 2018

zkbot referenced this in commit c31718fea2 on May 8, 2018

zkbot referenced this in commit 01b1962cb6 on May 8, 2018

zkbot referenced this in commit b1d2a69908 on May 8, 2018

PastaPastaPasta referenced this in commit 321444f034 on Jul 18, 2020

PastaPastaPasta referenced this in commit e6d6f12436 on Jul 19, 2020

PastaPastaPasta referenced this in commit db747ea384 on Jul 19, 2020

zkbot referenced this in commit 43ac2062f9 on Oct 28, 2020

zkbot referenced this in commit 84a5830aaa on Nov 9, 2020

UdjinM6 referenced this in commit 95e81d6954 on Dec 25, 2020

practicalswift deleted the branch on Apr 10, 2021

gades referenced this in commit cc45d2d8ad on Mar 3, 2022

DrahtBot locked this on Aug 18, 2022