Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions best_practices.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@

<b>Pattern 1: Require explicit runtime configuration for backend/service endpoints and fail fast when required environment variables or CLI arguments are missing, rather than shipping defaults or vendor-specific endpoints. Ensure docs and examples stay provider-agnostic and do not include internal/third-party service URLs.
</b>

Example code before:
```
private static final String DEFAULT_ENDPOINT = "https://vendor.example.com";

String endpoint = Environment.get("BACKEND_URL", DEFAULT_ENDPOINT);
// continues with default even when not configured
```

Example code after:
```
String endpoint = Environment.get("BACKEND_URL");
if (endpoint == null || endpoint.trim().isEmpty()) {
throw new IllegalStateException(
"Backend URL not configured. Please set BACKEND_URL.");
}
endpoint = endpoint.trim();
```

<details><summary>Examples for relevant past discussions:</summary>

- https://github.com/guacsec/trustify-da-java-client/pull/211#discussion_r2536810895
- https://github.com/guacsec/trustify-da-java-client/pull/211#discussion_r2537678862
</details>


___

<b>Pattern 2: When introducing a new configuration key/behavior that supersedes a legacy one, enforce deterministic precedence (new overrides old) and migrate examples/docs to primarily show the new key; only mention the legacy key as backwards compatibility. In code, centralize these keys as constants and implement the precedence logic once.
</b>

Example code before:
```
// scattered literals and unclear precedence
var node = json.get("legacyIgnore");
if (node == null || node.isEmpty()) {
node = json.get("newIgnore");
}
```

Example code after:
```
// constants + explicit precedence: new wins
var node = json.get(IGNORE_NEW);
if (node == null || node.isEmpty()) {
node = json.get(IGNORE_LEGACY);
}
if (node == null || node.isEmpty()) return Set.of();
```

<details><summary>Examples for relevant past discussions:</summary>

- https://github.com/guacsec/trustify-da-java-client/pull/205#discussion_r2526224893
- https://github.com/guacsec/trustify-da-java-client/pull/205#discussion_r2526228973
- https://github.com/guacsec/trustify-da-java-client/pull/205#discussion_r2526232347
- https://github.com/guacsec/trustify-da-java-client/pull/205#discussion_r2526242594
- https://github.com/guacsec/trustify-da-java-client/pull/205#discussion_r2526258368
</details>


___

<b>Pattern 3: Prefer failing with exceptions (and include stderr/context) over merely logging when an external tool/process invocation or required precondition fails, so callers get actionable failures. Use logging for diagnostics, not as a substitute for error handling.
</b>

Example code before:
```
String out = Operations.run(cmd);
if (out == null) {
LOG.warning("Tool failed, continuing");
return "";
}
```

Example code after:
```
ProcessResult r = Operations.runWithResult(cmd);
if (r.exitCode() != 0) {
throw new IOException("Tool failed: " + r.stderr());
}
return r.stdout();
```

<details><summary>Examples for relevant past discussions:</summary>

- https://github.com/guacsec/trustify-da-java-client/pull/167#discussion_r2228021393
</details>


___

<b>Pattern 4: Keep documentation and code comments minimal, accurate, and non-redundant: avoid repeating the same environment-variable setup in every example, remove outdated/irrelevant comments, and ensure parameter names match their Javadoc. Prefer a single “export once” step and then concise usage examples.
</b>

Example code before:
```
# Example 1
export BACKEND_URL=http://localhost:8080
tool cmd1

# Example 2
export BACKEND_URL=http://localhost:8080
tool cmd2
```

Example code after:
```
# Configure once
export BACKEND_URL=http://localhost:8080

# Examples
tool cmd1
tool cmd2
```

<details><summary>Examples for relevant past discussions:</summary>

- https://github.com/guacsec/trustify-da-java-client/pull/211#discussion_r2537683064
- https://github.com/guacsec/trustify-da-java-client/pull/211#discussion_r2537686339
- https://github.com/guacsec/trustify-da-java-client/pull/127#discussion_r2047246406
- https://github.com/guacsec/trustify-da-java-client/pull/114#discussion_r2031484262
</details>


___
20 changes: 10 additions & 10 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<jackson-annotations.version>2.21</jackson-annotations.version>
<jakarta.annotation-api.version>2.1.1</jakarta.annotation-api.version>
<jakarta.mail.version>2.0.5</jakarta.mail.version>
<cyclonedx.version>11.0.1</cyclonedx.version>
<cyclonedx.version>12.0.1</cyclonedx.version>
<tomlj.version>1.1.1</tomlj.version>
<!-- Testing Dependencies -->
<assertj.version>3.27.7</assertj.version>
Expand All @@ -39,21 +39,21 @@
<maven-gpg-plugin.version>3.2.8</maven-gpg-plugin.version>
<maven-help-plugin.version>3.5.1</maven-help-plugin.version>
<maven-install-plugin.version>3.1.1</maven-install-plugin.version>
<maven-jar-plugin.version>3.3.0</maven-jar-plugin.version>
<maven-jar-plugin.version>3.5.0</maven-jar-plugin.version>
<maven-javadoc-plugin.version>3.12.0</maven-javadoc-plugin.version>
<maven-release-plugin.version>3.0.0</maven-release-plugin.version>
<maven-resources-plugin.version>3.3.1</maven-resources-plugin.version>
<maven-site-plugin.version>4.0.0-M6</maven-site-plugin.version>
<maven-source-plugin.version>3.2.1</maven-source-plugin.version>
<maven-surefire-plugin.version>3.5.3</maven-surefire-plugin.version>
<maven-shade-plugin.version>3.4.1</maven-shade-plugin.version>
<build-helper-maven-plugin.version>3.4.0</build-helper-maven-plugin.version>
<maven-shade-plugin.version>3.6.1</maven-shade-plugin.version>
<build-helper-maven-plugin.version>3.6.0</build-helper-maven-plugin.version>
<extra-enforcer-rules.version>1.6.2</extra-enforcer-rules.version>
<flatten-maven-plugin.version>1.4.1</flatten-maven-plugin.version>
<jacoco-maven-plugin.version>0.8.10</jacoco-maven-plugin.version>
<flatten-maven-plugin.version>1.7.3</flatten-maven-plugin.version>
<jacoco-maven-plugin.version>0.8.14</jacoco-maven-plugin.version>
<junit-platform-maven-plugin.version>1.1.8</junit-platform-maven-plugin.version>
<license-maven-plugin.version>4.1</license-maven-plugin.version>
<pitest-maven.version>1.13.2</pitest-maven.version>
<pitest-maven.version>1.22.1</pitest-maven.version>
<pitest-junit5-plugin.version>1.1.2</pitest-junit5-plugin.version>
<versions-maven-plugin.version>2.15.0</versions-maven-plugin.version>
<maven-failsafe-plugin.version>3.5.3</maven-failsafe-plugin.version>
Expand Down Expand Up @@ -425,7 +425,7 @@ limitations under the License.]]>
<exclude>io/github/guacsec/trustifyda/api/*</exclude>
<exclude>io/github/guacsec/trustifyda/api/serialization/*</exclude>
<exclude>io/github/guacsec/trustifyda/exception/*</exclude>
<!-- This one excluding ExhortApi implementation calss from coverage report as it's not tested by surefire plugin, but with junit-platform-maven-plugin -->
<!-- This one excluding ExhortApi implementation class from coverage report as it's not tested by surefire plugin, but with junit-platform-maven-plugin -->
<exclude>io/github/guacsec/trustifyda/impl/*</exclude>
<exclude>io/github/guacsec/trustifyda/logging/*</exclude>
<exclude>io/github/guacsec/trustifyda/image/ImageUtils.class</exclude>
Expand Down Expand Up @@ -778,7 +778,7 @@ limitations under the License.]]>
<reflowLongStrings>true</reflowLongStrings> <!-- optional -->
</googleJavaFormat>
<licenseHeader>
<file>${project.basedir}/license-header</file> -->
<file>${project.basedir}/license-header</file>
</licenseHeader>
<removeUnusedImports />
<formatAnnotations />
Expand Down Expand Up @@ -851,7 +851,7 @@ limitations under the License.]]>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<version>5.9.1</version>
<version>${junit-jupiter.version}</version>
</dependency>
</dependencies>
<configuration>
Expand Down
Loading