diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..32d25cc --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,7 @@ +# All files in root dir and folders starting with . +# Mostly just protecting the touchy configuration things +/*.* @LakeEffectRobotics/reviewer +config/checkstyle/** @LakeEffectRobotics/reviewer + +# Optionally could add individual owners for source files below +# But would require them to review all PRs on those files diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..8401a78 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,17 @@ +## Changes +Description of your changes + +Include refernces to any issues you address, using closing words if they're resolved. + +## Testing +- [ ] Tested in Simulation (remove if not applicable) +- [ ] Tested on Robot + +Describe your testing here + + +## Checklist +- [ ] Code Builds +- [ ] Checkstyle Passes +- [ ] Code generates no new warnings +- [ ] Appropriately tested diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml new file mode 100644 index 0000000..7ac0dd9 --- /dev/null +++ b/.github/workflows/gradle.yml @@ -0,0 +1,98 @@ +# This workflow uses actions that are not certified by GitHub. +# They are provided by a third-party and are governed by +# separate terms of service, privacy policy, and support +# documentation. +# This workflow will build a Java project with Gradle and cache/restore any dependencies to improve the workflow execution time +# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-java-with-gradle + +name: Java CI with Gradle + +on: [push, pull_request] + +jobs: + build: + + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v4 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + cache: gradle + + - name: Make Gradlew executable + run: chmod +x ./gradlew + + # Configure Gradle for optimal use in GitHub Actions, including caching of downloaded dependencies. + # See: https://github.com/gradle/actions/blob/main/setup-gradle/README.md + - name: Setup Gradle + uses: gradle/actions/setup-gradle@af1da67850ed9a4cedd57bfd976089dd991e2582 # v4.0.0 + + - name: Build with Gradle Wrapper + run: ./gradlew build -x checkstyleMain + + # NOTE: The Gradle Wrapper is the default and recommended way to run Gradle (https://docs.gradle.org/current/userguide/gradle_wrapper.html). + # If your project does not have the Gradle Wrapper configured, you can use the following configuration to run Gradle with a specified version. + # + # - name: Setup Gradle + # uses: gradle/actions/setup-gradle@af1da67850ed9a4cedd57bfd976089dd991e2582 # v4.0.0 + # with: + # gradle-version: '8.9' + # + # - name: Build with Gradle 8.9 + # run: gradle build + + checkstyle-passfail: + runs-on: ubuntu-latest + needs: build + permissions: + contents: read + checks: write + + steps: + - uses: actions/checkout@v4 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + cache: gradle + + - name: Make Gradlew executable + run: chmod +x ./gradlew + + - name: Run Checkstyle + run: ./gradlew checkstyleMain + + checkstyle-report: + runs-on: ubuntu-latest + needs: build + permissions: + contents: read + checks: write + + steps: + - uses: actions/checkout@v4 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + cache: gradle + + - name: Make Gradlew executable + run: chmod +x ./gradlew + + - name: Run check style + uses: nikitasavinov/checkstyle-action@master + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + reporter: 'github-pr-check' + tool_name: 'testtool' + checkstyle_config: 'config/checkstyle/checkstyle.xml' + properties_file: 'config/checkstyle/github.properties' diff --git a/.vscode/extensions.json b/.vscode/extensions.json new file mode 100644 index 0000000..fcc67e7 --- /dev/null +++ b/.vscode/extensions.json @@ -0,0 +1,5 @@ +{ + "recommendations": [ + "shengchen.vscode-checkstyle" + ] +} \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json index 612cdd0..f953e76 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -56,5 +56,7 @@ "edu.wpi.first.math.proto.*", "edu.wpi.first.math.**.proto.*", "edu.wpi.first.math.**.struct.*", - ] + ], + "java.checkstyle.configuration": "${workspaceFolder}\\config\\checkstyle\\checkstyle.xml", + "editor.tabSize": 4 } diff --git a/build.gradle b/build.gradle index c6fea4f..c50e96c 100644 --- a/build.gradle +++ b/build.gradle @@ -1,6 +1,7 @@ plugins { id "java" id "edu.wpi.first.GradleRIO" version "2025.2.1" + id 'checkstyle' } java { @@ -8,6 +9,29 @@ java { targetCompatibility = JavaVersion.VERSION_17 } +checkstyle { + toolVersion = "10.12.4" + maxWarnings = 10 + // Run with higher severity on github actions pipeline + if(System.getenv("GITHUB_ACTIONS")){ + configProperties = [ + "severity": "error" + ] + } else { + configProperties = [ + "severity": "warning" + ] + } +} + +tasks.withType(Checkstyle) { + reports { + xml.required = true + html.required = true + // html.stylesheet = resources.text.fromFile('config/xsl/checkstyle-custom.xsl') + } +} + def ROBOT_MAIN_CLASS = "frc.robot.Main" // Define my targets (RoboRIO) and artifacts (deployable files) diff --git a/config/checkstyle/Style Guide.md b/config/checkstyle/Style Guide.md new file mode 100644 index 0000000..2ffdd89 --- /dev/null +++ b/config/checkstyle/Style Guide.md @@ -0,0 +1,130 @@ +# Style Guide + +**Why a style guide?** +Consistent styling is important to make it easier to switch between code written by different people. If we all use different styles, it starts getting harder to quickly understand why a subsystem isn't moving, or how to fix that auto, especially when under pressure at events. + +**Why did we choose X?** +The style guide is generally based on how we've written robot code in the past. None of the rules are truly set in stone between seasons, but generally we limit changes during a season to avoid constantly fixing the code to match. + +**Why the automatic checks?** +The automatic checker (called a Linter) means that you can see whether your code matches the style requirements in real time, and makes it much easier to fix minor issues as they come up. It also makes code review easier, as we don't need to manually nitpick minor style changes when trying to merge in large chunks. + +**Why does Github fail when VSCode builds fine?** The baseline severity in VSCode is set to warning, to avoid minor style errors causing stress during limited testing time or at competition. It is raised to error on Github to make sure sure code gets the extra checks before merging it. + +## Indentation +Indentation is standardized at 4 spaces (VSCode will automatically do this when you press tab). Standardizing indentation ensures that code copied between files will always be properly indented, and code within a file will be easier to read. 4 spaces was chosen as it's the standard for most Java styleguides. + +## Block Statements +Block statements (if, for, while) are written with the opening curly brace on the same line, and the closing curly brace on it's own line. + +Else/else if statements should start on the same line as the closing bracket. A space is required before the else keyword. No requirements exist for spacing around the parentheses (yet). + +```java +if (i == 0) { + // Do Stuff +} else if (i == 1) { + // Other Stuff +} +``` + +Inline if statements are allowed if they are on a single line. If the code is long enough that it should be wrapped, brackets must bet added. While this is annoying, it greatly reduces the potential for errors to be made later when editing that code + +```java +// Bad +if (i == 0) + i++; +// OK +if (i == 0) i++; +// Good +if (i == 0){ + i++; +} +``` + +## Naming +Consistent naming is one of the most important parts of styling. When things are named right, it's much easier to find what you need and understand what you're reading. + +### Classes +Classes, Enums, and other types should be named with `UpperCamelCase` (also called `PascalCase`). The first letter is capitalized, as well as the first letter of each word. No underscores should be used. + +Subsystem classes should be named after the mechanism (e.g. `Drivetrain`, `Intake`) + +Commands should be suffixed with `Command` (e.g. `DriveCommand`, `IntakeCommand`) + +Often, commands which do something automatically are named with both an `Auto` prefix and `Command` suffix (e.g. `AutoDriveCommand`, `AutoIntakeCommand`). This is less of a requirement, but helps keep things easy to understand. + +### Constants +Constants are variables defined as `final` or `static final`. They are used to store data which doesn't change while running, such as physical properties. + +To make it clear that a variable is a constant, it should be named with `CAPITAL_CASE`. All letters are capitalized, and words are separated by underscores. + +Constants may also be prefixed with a `k`, as for some reason the scientific community has decided that `k` stands for konstant. This is generally seen in constants use for physics equations like `kP`, `kI`, `kD`, `kG`. + +### All other variables +All other variables (members, locals, parameters) should be named with `camelCase`. The first letter is lowercase, and the first letter of each following word is capitalized. There are no undescores. + +### Unused formats +We don't use `snake_case`, for anything, it's mostly a python/embedded style. + +While the docs often prefix variables with `m_`, we don't use it as there isn't that much conflict between member and local variables in robot code. + +## Whitespace +### Tokens +Some tokens should not be preceeded by whitespace. Commas, colons, and semicolons should be placed immediately following the previous character. This rule also applies to the `++` and `--` operators. + +Typecasts are written in a way that minimizes whitespace inside parentheses. (e.g. `(int)f` instead of `( int ) f`. `(int) f` is acceptable) + +### Functions +Parentheses for function declarations and calls are placed next to the name without whitespace. (e.g. `func()` is correct, `func ()` is incorrect) + +### Declarations +Declarations in a class should be separated by one or more blank lines. This pads things out a bit so it's easier to read. You can put multiple variable declarations back-to-back, and it's up to the programmer to space them out based on context. + +```java +// Variables grouped together +int kI; +int kP; +int kD; + +//Blank line before/between functions +public void func() { + +} + +public void func2() { + +} +``` + +## String re-use +If the same string is used 3 or more times, in a file, then a dedicated constant should be created for it. This ensures that if can't be changed in one place while missing another. + +The strings `"\n"` and `"\t"` are excempt from this rule. + +## Misc +Only one statement is on a line (e.g. `a = 0; b = 1;` should be split across two lines) + +Semicolons should not be placed following a closing curly bracket. + +Annotations (`@Override`) are placed above the corresponding declaration, with one annotation per line. + +The `x` and `b` in literals are lowercase (e.g. `0xBEEF`, `0b101010`) + +Array brackets are placed directly next to the type, not variable (e.g. `int[] arr`) + +## Footguns +These rules will be treated like errors if broken, since they're checking for things that likely *are* mistakes. + +### Empty inline if statements +An if statement in the format `if(stmt);` without a condition before the semicolon is very likely a typo where the semicolon should be after the next line. If you do want an empty if statement, write it like `if(stmt) {}` instead. + +### Switch break/default +Switch conditions must end with a `break` statement, preventing accidental fallthrough. If you do want a fallthrough, just place a `// fallthrough` comment on the last line. + +The `default` case of a switch must be the last case, as nothing after it will be run. Java allows you to do this, but it's really just a footgun. + +### Assignment in if/while statements +While there are a few reasons to have an assignment in an if/while statement (`if (a = b)`), you almost certainly wanted the comparison (`if (a == b)`) + +### String equality +Java lets you compare strings with `a == b`, but that's prone to giving the wrong behaviour. Instead use `a.equals(b)`. \ No newline at end of file diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml new file mode 100644 index 0000000..3ad19a9 --- /dev/null +++ b/config/checkstyle/checkstyle.xml @@ -0,0 +1,175 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/config/checkstyle/github.properties b/config/checkstyle/github.properties new file mode 100644 index 0000000..dcc7c8e --- /dev/null +++ b/config/checkstyle/github.properties @@ -0,0 +1 @@ +severity=error \ No newline at end of file diff --git a/src/main/java/frc/robot/Main.java b/src/main/java/frc/robot/Main.java index fe215d7..25fa2ae 100644 --- a/src/main/java/frc/robot/Main.java +++ b/src/main/java/frc/robot/Main.java @@ -7,9 +7,10 @@ import edu.wpi.first.wpilibj.RobotBase; public final class Main { - private Main() {} + private Main() { + } - public static void main(String... args) { - RobotBase.startRobot(Robot::new); - } + public static void main(String... args) { + RobotBase.startRobot(Robot::new); + } } diff --git a/src/main/java/frc/robot/Robot.java b/src/main/java/frc/robot/Robot.java index be21731..dc486ae 100644 --- a/src/main/java/frc/robot/Robot.java +++ b/src/main/java/frc/robot/Robot.java @@ -9,64 +9,64 @@ import edu.wpi.first.wpilibj2.command.CommandScheduler; public class Robot extends TimedRobot { - private Command m_autonomousCommand; + private Command autonomousCommand; - private final RobotContainer m_robotContainer; + private final RobotContainer robotContainer; - public Robot() { - m_robotContainer = new RobotContainer(); - } + public Robot() { + robotContainer = new RobotContainer(); + } - @Override - public void robotPeriodic() { - CommandScheduler.getInstance().run(); - } + @Override + public void robotPeriodic() { + CommandScheduler.getInstance().run(); + } - @Override - public void disabledInit() {} + @Override + public void disabledInit() {} - @Override - public void disabledPeriodic() {} + @Override + public void disabledPeriodic() {} - @Override - public void disabledExit() {} + @Override + public void disabledExit() {} - @Override - public void autonomousInit() { - m_autonomousCommand = m_robotContainer.getAutonomousCommand(); + @Override + public void autonomousInit() { + autonomousCommand = robotContainer.getAutonomousCommand(); - if (m_autonomousCommand != null) { - m_autonomousCommand.schedule(); + if (autonomousCommand != null) { + autonomousCommand.schedule(); + } } - } - @Override - public void autonomousPeriodic() {} + @Override + public void autonomousPeriodic() {} - @Override - public void autonomousExit() {} + @Override + public void autonomousExit() {} - @Override - public void teleopInit() { - if (m_autonomousCommand != null) { - m_autonomousCommand.cancel(); + @Override + public void teleopInit() { + if (autonomousCommand != null) { + autonomousCommand.cancel(); + } } - } - @Override - public void teleopPeriodic() {} + @Override + public void teleopPeriodic() {} - @Override - public void teleopExit() {} + @Override + public void teleopExit() {} - @Override - public void testInit() { - CommandScheduler.getInstance().cancelAll(); - } + @Override + public void testInit() { + CommandScheduler.getInstance().cancelAll(); + } - @Override - public void testPeriodic() {} + @Override + public void testPeriodic() {} - @Override - public void testExit() {} + @Override + public void testExit() {} } diff --git a/src/main/java/frc/robot/RobotContainer.java b/src/main/java/frc/robot/RobotContainer.java index de2c9d0..5141a26 100644 --- a/src/main/java/frc/robot/RobotContainer.java +++ b/src/main/java/frc/robot/RobotContainer.java @@ -8,13 +8,13 @@ import edu.wpi.first.wpilibj2.command.Commands; public class RobotContainer { - public RobotContainer() { - configureBindings(); - } + public RobotContainer() { + configureBindings(); + } - private void configureBindings() {} + private void configureBindings() {} - public Command getAutonomousCommand() { - return Commands.print("No autonomous command configured"); - } + public Command getAutonomousCommand() { + return Commands.print("No autonomous command configured"); + } }