Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add current O3DE coding standards #5

Merged
merged 2 commits into from
Jun 17, 2021
Merged

Add current O3DE coding standards #5

merged 2 commits into from
Jun 17, 2021

Conversation

rgba16f
Copy link
Contributor

@rgba16f rgba16f commented Jun 14, 2021

@rgba16f
Add current Amazon O3DE coding standards
181d5cb 
Signed-off-by: rgba16f <82187279+rgba16f@users.noreply.github.com>
@netlify
Copy link

netlify bot commented Jun 14, 2021 •
edited
Loading

✔️ Deploy Preview for compassionate-kare-0aea30 ready!

🔨 Explore the source changes: 98aa6b2

🔍 Inspect the deploy log: https://app.netlify.com/sites/compassionate-kare-0aea30/deploys/60c8e334327c3c0008c6e245

😎 Browse the preview: https://deploy-preview-5--compassionate-kare-0aea30.netlify.app

amzn-pratikpa
amzn-pratikpa reviewed Jun 14, 2021
View reviewed changes
governance/Coding-Standards-and-Style-Guide.md
* Note: Nested blocks are usually started by entering a new scope, i.e. whenever you would have used a brace, but there are other situations (see examples below).
* Indent all lines of a block by the same amount (see "Make lines reasonably long" for advice on continued lines).

**Uncrustify**: Will automatically format to these guidelines.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we requiring external developers to use Uncrustify?

Copy link

Choose a reason for hiding this comment

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

Clang-tidy is being used, since we use Clang on several platforms and it's embedded in Visual Studio so it formats code as you type.

OBWANDO
OBWANDO approved these changes Jun 15, 2021
View reviewed changes
Copy link
Contributor

@OBWANDO OBWANDO left a comment

Choose a reason for hiding this comment

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

Just need to remove 11.9 as it doesn't apply on next pass.

@rgba16f
minor cleanup
98aa6b2 
Signed-off-by: rgba16f <82187279+rgba16f@users.noreply.github.com>
@amzn-pratikpa amzn-pratikpa merged commit dadcd9f into o3de:main Jun 17, 2021
@rgba16f rgba16f deleted the AddCodingStandard branch June 17, 2021 21:31
@amzn-pratikpa amzn-pratikpa mentioned this pull request Jul 14, 2021
Closed
@OBWANDO OBWANDO changed the title Add current Amazon O3DE coding standards Add current O3DE coding standards Jul 14, 2021
@lemonade-dm
Copy link
Contributor

A couple of issues.
Is the formatting on the page isn't correct: It looks like it is markdown exported from confluence, so it contains some of it's idiosyncrasies: 181d5cb?short_path=d59db2a#diff-
d59db2a1a0d380027161eb5760b660573dcdf46dba5d8e5a6706ebb817ea64af

The minimum clang version on any support platform 6.0.0(Linux) and not 5.0.0.
Mac and iOS is minimum Xcode 12,(which is based on LLVM 10.0.0).
Android is based on NDK 22.1 which is Clang 11.0.5

@nemerle
Copy link

nemerle commented Jul 16, 2021

A few notes:

  • pre-formatted code is not displaying correctly ( all examples following links Expand source )
  • the link Using Uncrustify to beautify Lumberyard code points to the non-existent address https://github.com/o3de/sig-core/blob/main/display/lmbr/Using+Uncrustify+to+beautify+Lumberyard+code
  • Gems System Documentation,Copyright Header Requirements links point to another non-existent address.

In section 5.1 location of #pragma once in the header could be more specific, some files that mix #ifdef header guards have it after the guard:

#ifndef CRYINCLUDE_EDITOR_2DVIEWPORT_H
#define CRYINCLUDE_EDITOR_2DVIEWPORT_H

#pragma once

others have it at the end of file:

#ifndef AZCORE_DRILLER_H
#define AZCORE_DRILLER_H
// FILE CONTENTS HERE
#endif // AZCORE_DRILLER_H
#pragma once

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Infurael Infurael Infurael left review comments

@amzn-pratikpa amzn-pratikpa amzn-pratikpa approved these changes

@OBWANDO OBWANDO OBWANDO approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rgba16f @lemonade-dm @nemerle @OBWANDO @amzn-pratikpa @Infurael