Skip to content

Commit

Permalink
Merge pull request #747 from shinybrar/main
Browse files Browse the repository at this point in the history
fix(style): linted code of conduct and contribution page
  • Loading branch information
at88mph authored Nov 26, 2024
2 parents 19f2608 + db59e63 commit d7f2eb8
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.commit.check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ jobs:
commit-signoff: false
merge-base: true
job-summary: true
pr-comments: ${{ github.event_name == 'pull_request' }}
pr-comments: false
1 change: 0 additions & 1 deletion CODE_OF_CONDUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,3 @@ We strive to:
1. Reporting Guidelines – We should assume good faith in our actions; however, issues that cannot be resolved that are addressed in this code of conduct should be sent to the Group Leader of the CADC: [email protected]

1. This Code draws on the Apache Software Foundation Code of Conduct: [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html) and the National Research Council Canada (NRC) Code of Conduct: [Code of Conduct](https://nrc.canada.ca/en/corporate/values-ethics/code-conduct).

6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ All java libraries are published to Maven Central under the <a href="https://rep
Java dependencies should be carefully curated to include direct dependencies on java libs and only rely on transitive dependencies for indirect dependencies. Exception: for external libraries (e.g. log4j, xerces and jdom2 xml parsers, spring-jdbc) it is preferrable to use the version specified by the core `cadc-util` library and _not_ declare any dependency on these even though they are used directly by the code. This allows for easier version upgrade (e.g. for security issues) and consistent use. For other external dependencies introduced elsewhere, care should be taken to consider future update scenarios and try to minimise the pain necessary to carry them out. The dependency list in all build files should be curated to remove unused dependencies and sometimes even block transitive dependencies from the runtime classpath.

#### Code Style
Java coding style is specified and validated using `checkstyle` via the <a href="https://github.com/opencadc/core/tree/main/cadc-quality">cadc-quality</a> library. Each repository includes this library in the test classpath and configures checkstyle via the repository `opencadc.gradle` file. Developers can run checkstyle locally using `gradle checkstyleMain` (we currently do not enforce checkstyle on test code **TBD**) to make sure they comply and **should** use the checkstyle configuration from `cadc-quality` to configure their development environment (IDE) where possible.
Java coding style is specified and validated using `checkstyle` via the <a href="https://github.com/opencadc/core/tree/main/cadc-quality">cadc-quality</a> library. Each repository includes this library in the test classpath and configures checkstyle via the repository `opencadc.gradle` file. Developers can run checkstyle locally using `gradle checkstyleMain` (we currently do not enforce checkstyle on test code **TBD**) to make sure they comply and **should** use the checkstyle configuration from `cadc-quality` to configure their development environment (IDE) where possible.

This is currently checkstyle-8.2 -- pretty old -- and has some defiencies that need to be addressed. In addition, `gradle javadoc` must have no errors and ideally no warnings (technical debt: most older code has lots of warnings).
This is currently checkstyle-8.2 -- pretty old -- and has some defiencies that need to be addressed. In addition, `gradle javadoc` must have no errors and ideally no warnings (technical debt: most older code has lots of warnings).

The current code style checking really only covers small scale syntax and not larger scale code organisation and design. General principles:
* clear code so comments are not needed or can be minimised
Expand All @@ -52,7 +52,7 @@ The current code style checking really only covers small scale syntax and not la
#### Continuous Integration
Continuous Integration (CI) is used to validate pull requests in github (when PR is opened or new commits added). Reviewer(s) will usually not look at a PR when CI fails; it is the responsibility of the person who created the PR to fix CI issues or add a comment explaining it. For example, if a PR requires a new version of a library that has not been published yet, add a comment explaining that and then the reviewer can trigger the CI again once the library is published.

Github CI is not currently used to publish java libraries or docker images.
Github CI is not currently used to publish java libraries or docker images.

#### Publishing
Java libraries are published to Maven Central manually by a CADC staff member.
Expand Down
35 changes: 34 additions & 1 deletion skaha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,41 @@ pip3 install --user pre-commit
pre-commit install --hook-type commit-msg
```

This will install the pre-commit hooks to run on every commit. If you want to run the hooks manually, you can run the following command:
This will install the pre-commit hooks to run on every commit, even the checks defined under `./gradlew check`. If you want to run the hooks manually, you can run the following command:

```
pre-commit run --all-files
```

## Code Formatting

Skaha uses `spotless` to enforce Java Code Formatting standards. To format the code, run the following command:

```bash
cd science-platform/skaha
./gradlew spotlessApply
```

You can find the spotless configuration in the `spotless` block in the `build.gradle` file.

```java
spotless {
java {
// Interpret all files as utf-8
encoding 'UTF-8'
// Only require formatting of files that diff from main
ratchetFrom 'origin/main'
// Use the default importOrder configuration
importOrder()
// Remove unused imports
removeUnusedImports()
// Google Java Format, Android Open Source Project style which uses 4 spaces for indentation
palantirJavaFormat('2.50.0').formatJavadoc(true)
// Format annotations on a single line
formatAnnotations()
}
}
check.dependsOn spotlessCheck
```

We specificly use the [Palantir Java Format](https://github.com/palantir/palantir-java-format), which is a a modern, lambda-friendly, 120 character Java formatter focused on readability and derived from Google Java Format.

0 comments on commit d7f2eb8

Please sign in to comment.