Skip to content

Conversation

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Apr 13, 2018

Suggested formatting for namespaces in the developer guide is currently inconsistent. This commit updates the developer guide and clang-format configuration to consistently put "namespace" and opening/closing braces on the same line. Example:

namespace boost {
namespace signals2 {
class connection;
} // namespace signals2
} // namespace boost

Currently the Source code organization section has an example like the one above, but the Coding style section example and description put a newline between the opening "namespace foo" and brace (but oddly no newline between closing namespace and brace).

Avoiding newlines before namespace opening braces makes nested declarations less verbose and also avoids asymmetry with closing braces. It's also a common style used in our own and other codebases:

Suggested formatting for namespaces in the developer guide is currently
inconsistent. This commit updates the developer guide and clang-format
configuration to consistently put "namespace" and opening/closing braces
on the same line. Example:

```c++
namespace boost {
namespace signals2 {
class connection;
} // namespace signals2
} // namespace boost
```

Currently the "Source code organization" section has an example like the one
above, but the "Coding style" section example and description put a newline
between the opening "namespace foo" and brace (but oddly no newline between
closing namespace and brace).

Avoiding newlines before namespace opening braces makes nested declarations
less verbose and also avoids asymmetry with closing braces. It's also a
common style used in other codebases:

* https://google.github.io/styleguide/cppguide.html#Namespaces
* https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Classes
* https://llvm.org/docs/CodingStandards.html#namespace-indentation
@practicalswift
Copy link
Contributor

Concept ACK

BraceWrapping:
AfterClass: true
AfterFunction: true
AfterStruct: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

structs had their braces on the same line previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

structs had their braces on the same line previously?

Good catch, this was a mistake.

@maflcko
Copy link
Member

maflcko commented Apr 14, 2018

Fine with me, since this seems a common theme, but I feel like the other settings in the clang-format file should not be toggled.

@ryanofsky ryanofsky force-pushed the pr/namespace branch 3 times, most recently from 681dea1 to cd0e1e9 Compare April 14, 2018 14:18
@ryanofsky
Copy link
Contributor Author

ryanofsky commented Apr 14, 2018

but I feel like the other settings in the clang-format file should not be toggled

Fixed now, this was a mistake. I misread the documentation which didn't mention there was difference between classes and structs.

Updated 681dea1 -> cd0e1e9 (pr/namespace.1 -> pr/namespace.2)

@maflcko
Copy link
Member

maflcko commented Apr 14, 2018

utACK cd0e1e9

@Empact
Copy link
Contributor

Empact commented Apr 15, 2018

utACK cd0e1e9

@jnewbery
Copy link
Contributor

utACK cd0e1e9

@promag
Copy link
Contributor

promag commented Apr 16, 2018

utACK cd0e1e9.

@maflcko maflcko merged commit cd0e1e9 into bitcoin:master Apr 17, 2018
maflcko pushed a commit that referenced this pull request Apr 17, 2018
cd0e1e9 Fix inconsistent namespace formatting guidelines (Russell Yanofsky)

Pull request description:

  Suggested formatting for namespaces in the developer guide is currently inconsistent. This commit updates the developer guide and clang-format configuration to consistently put "namespace" and opening/closing braces on the same line. Example:

  ```c++
  namespace boost {
  namespace signals2 {
  class connection;
  } // namespace signals2
  } // namespace boost
  ```

  Currently the [Source code organization](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#source-code-organization) section has an example like the one above, but the [Coding style](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style) section example and description put a newline between the opening "namespace foo" and brace (but oddly no newline between closing namespace and brace).

  Avoiding newlines before namespace opening braces makes nested declarations less verbose and also avoids asymmetry with closing braces. It's also a common style used in our own and other codebases:

  * https://google.github.io/styleguide/cppguide.html#Namespaces
  * https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Classes
  * https://llvm.org/docs/CodingStandards.html#namespace-indentation

Tree-SHA512: 507259478e1a7f6f96db386dd04eb25aa04294f533503fdd82368cf809c3ceaa20204b2cb6ae65322eb27446e5934c1aa1ccb6240ead12aa06b314af76f68139
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2020
cd0e1e9 Fix inconsistent namespace formatting guidelines (Russell Yanofsky)

Pull request description:

  Suggested formatting for namespaces in the developer guide is currently inconsistent. This commit updates the developer guide and clang-format configuration to consistently put "namespace" and opening/closing braces on the same line. Example:

  ```c++
  namespace boost {
  namespace signals2 {
  class connection;
  } // namespace signals2
  } // namespace boost
  ```

  Currently the [Source code organization](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#source-code-organization) section has an example like the one above, but the [Coding style](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style) section example and description put a newline between the opening "namespace foo" and brace (but oddly no newline between closing namespace and brace).

  Avoiding newlines before namespace opening braces makes nested declarations less verbose and also avoids asymmetry with closing braces. It's also a common style used in our own and other codebases:

  * https://google.github.io/styleguide/cppguide.html#Namespaces
  * https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Classes
  * https://llvm.org/docs/CodingStandards.html#namespace-indentation

Tree-SHA512: 507259478e1a7f6f96db386dd04eb25aa04294f533503fdd82368cf809c3ceaa20204b2cb6ae65322eb27446e5934c1aa1ccb6240ead12aa06b314af76f68139
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants