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.

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

Docker for compiling, linting, and testing #41

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

Conversation

StanleyTang17
Copy link
Contributor

@StanleyTang17 StanleyTang17 commented Mar 17, 2024

Description

What was completed, changed, or updated?

Added Docker for compiling, linting, and testing. Configured linting options and refactored tools script.

Files changed/added:

  • README.md - added instructions on how to use Docker
  • .gitignore - added ignore path for Docker linting output folder
  • .clang-tidy - configured clang-tidy linting rules
  • .clang-format - configured clang-format styling rules
  • .github/workflows/compile_test.yaml - modified to compile using the Docker script
  • .github/workflows/unit_test.yaml - modified to run tests using the Docker script
  • tools/Dockerfile - Docker file for building the Docker image
  • tools/run_docker.bash - Docker script for Linux and MacOS
  • tools/run_docker.ps1 - Docker script for Windows
  • tools/configs/compile_config.txt - default compile configurations
  • tools/configs/lint_config.txt - default lint configurations
  • tools/configs/static_analysis_ignore.txt - paths to files/folders that should be ignored by code linting (clang-tidy, clang-format, cppcheck)
  • tools/configs/test_config.txt - default test configurations
  • tools/scripts/clang-tidy.bash - script that runs clang-tidy in the Docker container
  • tools/scripts/clang-format.bash - script that runs clang-format in the Docker container
  • tools/scripts/cppcheck.bash - script that runs cppcheck in the Docker container
  • tools/scripts/static_analysis_files.bash - helper script that returns all files to be linted
  • tools/scripts/build.bash - helper script for building ZP binary and tests, derived from tools.bash
  • tools/scripts/test.bash - helper script for running tests, derived from tools.bash

Files removed:

  • Tools/tools.bash - split into build.bash and test.bash
  • Tools/tools.ps1 - removed since we are moving to the docker envrionment
  • Tools/default_config.txt - split into compile_config.txt and test_config.txt

Why was this done (if applicable)?

We decided to use Docker to create a user-friendly development workflow across all platforms. Linting rules were set to lint the code against our coding style standards. See Style Guidelines and clang-tidy/clang-format Rules in the PR for more details.

The Tools folder has also been renamed to conform to our newest naming conventions and restrucutred for better organization.


Testing

What manual tests were used to validate the code?

Manually ran all the docker script options and verified output.


What unit tests were used to validate the code?

N/A. This is an infra change.


Documentation

Milestone number and name: N/A

Link to Asana task:
https://app.asana.com/0/1203458353737758/1206459190171398/f
https://app.asana.com/0/1203458353737758/1206459190497129/f
https://app.asana.com/0/1203458353737758/1206459190171400/f
https://app.asana.com/0/1203458353737758/1207360178600434/f

Link to Confluence documentation:
Ugh, I don't have docs on these changes yet. At the very least, I need to add Docker instructions to our development environment setup page, but do I need to create detailed pages explaining how Docker and the scripts in this PR works as well? or would adding comments in the scripts suffice?


Reminders

  • Update documentation

Style Guidelines and clang-tidy/clang-format Rules

Column Width Per Line

clang-format rule:

ColumnLimit:     100

Variables

int youAreReadingThis = 0;

clang-tidy rule:

readability-identifier-naming.VariableCase: 'camelBack'

Constants

const int CONSTANT_VAR = 0;

clang-tidy rule:

readability-identifier-naming.ConstantCase: 'UPPER_CASE'

Pointers

int *integerPointer = nullptr;

clang-format rule:

PointerAlignment: Right

Classes

class ExampleClass {
    public:
        ... 
  
    protected: 
        ...
    
    private:
        ...
};

clang-tidy rule:

readability-identifier-naming.ClassCase: 'CamelCase'

clang-format rule:

IndentAccessModifiers: true
AccessModifierOffset: 0

Member Variables

class ExampleClass {
    public:
        bool isWiz_;
};

clang-tidy rules:

readability-identifier-naming.MemberCase: 'camelBack'
readability-identifier-naming.MemberSuffix: '_'

Constructors

ExampleClass::ExampleClass() : apple_pie {0}, banana_pies {0}, slices_left {0.0f} {}

// You can stack your initializing list. Just make sure they start in the same column
ExampleClass::ExampleClass(int num_apple_pies)
    : apple_pies {num_apple_pies},
      banana_pies {0} {
    slices_left = 6* num_apple_pies; // If an operation is required, preferred if initialization is within constructor
}

clang-format rules:

BreakConstructorInitializers: BeforeColon
PackConstructorInitializers: CurrentLine

Structs

struct StructName {
    ...
};
typedef struct NewType {
    ...
} NewType_t;

clang-tidy rules:

readability-identifier-naming.StructCase: 'CamelCase'
readability-identifier-naming.TypedefCase: 'CamelCase'
readability-identifier-naming.TypedefSuffix: '_t'

Enums

enum CheeseWiz_e {BACON, PIMENTO, TEX_MEX, LIGHT};

clang-tidy rules:

readability-identifier-naming.EnumCase: 'CamelCase'
readability-identifier-naming.EnumSuffix: '_e'

Case Statements

switch(a) {
    case 1: // Indent the cases 
        ... 
        break;
    case 2:
    case 3:
        ... 
        break;
    default: // Always include a default case
        break;
}

clang-format rules:

IndentCaseLabels: true
IndentCaseBlocks: true

Block Statement Braces

if (isCheeseWiz()) {
    // do something
}

clang-format rule:

BreakBeforeBraces: Attach

@StanleyTang17 StanleyTang17 added the enhancement New feature or request label Mar 17, 2024
@StanleyTang17 StanleyTang17 self-assigned this Mar 17, 2024
@StanleyTang17 StanleyTang17 changed the title Feature/infra/docker Docker for compiling and linting Mar 17, 2024
Tools/Dockerfile Outdated Show resolved Hide resolved
Tools/clang-format.bash Outdated Show resolved Hide resolved
Tools/cppcheck.bash Outdated Show resolved Hide resolved
Tools/clang-tidy.bash Outdated Show resolved Hide resolved
Tools/static_analysis_files.bash Outdated Show resolved Hide resolved
@StanleyTang17 StanleyTang17 marked this pull request as draft April 1, 2024 04:17
@StanleyTang17 StanleyTang17 marked this pull request as ready for review April 1, 2024 05:08
@StanleyTang17 StanleyTang17 changed the title Docker for compiling and linting Docker for compiling, linting, and testing Apr 1, 2024
Tools/Dockerfile Outdated Show resolved Hide resolved
Tools/HelperScripts/static_analysis_files.bash Outdated Show resolved Hide resolved
Tools/run_docker.bash Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants