Skip to content

Update guidelines based on recent discussions within CQWG #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

kadet1090
Copy link
Member

This PR updates guidelines based on our recent discussions within CQWG. The aim is to provide more unified review experience for both reviewers and PR authors. PR is split into commits so if we don't want some part of the PR we can remove some parts, or discuss them as separate PRs.

Important Changes

  1. Added informaion about auto to reviewers guide
  2. Added commit guidelines
  3. Added naming conventions
  4. Updated C++ practices to adhere to auto guidelines
  5. Updated C++ practices to prefer std::initialize_list over array as it does not require explicit count
  6. Removed C++11 comments from C++ practices as we use C++20

@@ -194,13 +230,29 @@ In this document the **bolded** text will indicate how important each suggestion


## Naming Things

Naming described in that section of the document refers mostly to newer parts of code. Some modules use different naming conventions,
while checking the code Reviewer should take into account surrounding code and first of all ensure consistency within the closes scope.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
while checking the code Reviewer should take into account surrounding code and first of all ensure consistency within the closes scope.
while checking the code Reviewer should take into account surrounding code and first of all ensure consistency within the closest scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants