sha tag build ids #2326

Merged
mfreeman451 merged 1 commit from refs/pull/2326/head into main 2025-10-16 05:38:06 +00:00
mfreeman451 commented 2025-10-16 05:35:28 +00:00 (Migrated from github.com)
Owner

Imported from GitHub pull request.

Original GitHub pull request: #1782
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1782
Original created: 2025-10-16T05:35:28Z
Original updated: 2025-10-16T05:38:09Z
Original head: carverauto/serviceradar:1776-bugbuilds-build-info-build-id-not-changing-with-bazel-builds
Original base: main
Original merged: 2025-10-16T05:38:06Z by @mfreeman451

PR Type

Enhancement, Bug fix


Description

  • Implement SHA-based build IDs for web and core images using digest values

  • Add dynamic build-info.json generation with web and core build IDs

  • Fix build info file handling in entrypoint script to prevent overwrites

  • Update sidebar to display separate web and core build identifiers


Diagram Walkthrough

flowchart LR
  A["workspace_status.sh"] -- "generates build metadata" --> B["BUILD.bazel"]
  B -- "creates build-info.json" --> C["web_image layers"]
  C -- "embedded in image" --> D["entrypoint-web.sh"]
  D -- "preserves build-info" --> E["Sidebar.tsx"]
  E -- "displays build IDs" --> F["UI"]

File Walkthrough

Relevant files
Enhancement
BUILD.bazel
Generate build-info.json from image digests and layer into web image

docker/images/BUILD.bazel

  • Split web_image_amd64 into base image and final image with build info
    layers
  • Add web_build_info_json genrule to generate build-info.json from image
    digests
  • Create four pkg_tar layers for build-info.json in different public
    directories
  • Extract short SHA hashes from web and core image digests for build IDs
+108/-1 
push_targets.bzl
Support custom digest labels for image push targets           

docker/images/push_targets.bzl

  • Update GHCR_PUSH_TARGETS to support custom digest labels
  • Modify declare_ghcr_push_targets to handle optional third tuple
    element
  • Set web image to use web_image_base_amd64.digest instead of default
+8/-3     
workspace_status.sh
Add version and timestamp variables to workspace status   

scripts/workspace_status.sh

  • Add STABLE_VERSION output from VERSION file or default to "dev"
  • Add optional STABLE_BUILD_ID output from BUILD_ID environment variable
  • Add BUILD_TIMESTAMP and BUILD_TIMESTAMP_COMPACT outputs with current
    UTC time
+18/-0   
build-info.json
Split build ID into separate web and core identifiers       

web/public/build-info.json

  • Change buildId field to separate webBuildId and coreBuildId fields
  • Update default values to "development" for build IDs
  • Set placeholder timestamps for development builds
+4/-3     
Sidebar.tsx
Display separate web and core build IDs in sidebar             

web/src/components/Sidebar.tsx

  • Add BuildInfo type with webBuildId and coreBuildId fields
  • Update build info display to show separate web and core build IDs
  • Simplify error handling in build info fetch logic
  • Display build timestamp when available
+24/-18 
Bug fix
entrypoint-web.sh
Preserve build-info.json during public asset flattening   

docker/compose/entrypoint-web.sh

  • Remove nested build-info.json before flattening public assets
  • Copy root build-info.json back to nested location after flattening
  • Apply same logic to both regular and standalone public directories
+8/-0     

Imported from GitHub pull request. Original GitHub pull request: #1782 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1782 Original created: 2025-10-16T05:35:28Z Original updated: 2025-10-16T05:38:09Z Original head: carverauto/serviceradar:1776-bugbuilds-build-info-build-id-not-changing-with-bazel-builds Original base: main Original merged: 2025-10-16T05:38:06Z by @mfreeman451 --- ### **PR Type** Enhancement, Bug fix ___ ### **Description** - Implement SHA-based build IDs for web and core images using digest values - Add dynamic `build-info.json` generation with web and core build IDs - Fix build info file handling in entrypoint script to prevent overwrites - Update sidebar to display separate web and core build identifiers ___ ### Diagram Walkthrough ```mermaid flowchart LR A["workspace_status.sh"] -- "generates build metadata" --> B["BUILD.bazel"] B -- "creates build-info.json" --> C["web_image layers"] C -- "embedded in image" --> D["entrypoint-web.sh"] D -- "preserves build-info" --> E["Sidebar.tsx"] E -- "displays build IDs" --> F["UI"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>BUILD.bazel</strong><dd><code>Generate build-info.json from image digests and layer into web image</code></dd></summary> <hr> docker/images/BUILD.bazel <ul><li>Split <code>web_image_amd64</code> into base image and final image with build info <br>layers<br> <li> Add <code>web_build_info_json</code> genrule to generate build-info.json from image <br>digests<br> <li> Create four pkg_tar layers for build-info.json in different public <br>directories<br> <li> Extract short SHA hashes from web and core image digests for build IDs</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80">+108/-1</a>&nbsp; </td> </tr> <tr> <td> <details> <summary><strong>push_targets.bzl</strong><dd><code>Support custom digest labels for image push targets</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> docker/images/push_targets.bzl <ul><li>Update <code>GHCR_PUSH_TARGETS</code> to support custom digest labels<br> <li> Modify <code>declare_ghcr_push_targets</code> to handle optional third tuple <br>element<br> <li> Set web image to use <code>web_image_base_amd64.digest</code> instead of default</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-4af33fe62caba04b6d479589c16cfb85babc39bae5c92595d4d4e31660738513">+8/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td> <details> <summary><strong>workspace_status.sh</strong><dd><code>Add version and timestamp variables to workspace status</code>&nbsp; &nbsp; </dd></summary> <hr> scripts/workspace_status.sh <ul><li>Add <code>STABLE_VERSION</code> output from VERSION file or default to "dev"<br> <li> Add optional <code>STABLE_BUILD_ID</code> output from BUILD_ID environment variable<br> <li> Add <code>BUILD_TIMESTAMP</code> and <code>BUILD_TIMESTAMP_COMPACT</code> outputs with current <br>UTC time</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-31442ee54466f451fa76cecc2f586dd19991fbe1eb88a7173afdeddb2b1a2b2c">+18/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td> <details> <summary><strong>build-info.json</strong><dd><code>Split build ID into separate web and core identifiers</code>&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> web/public/build-info.json <ul><li>Change <code>buildId</code> field to separate <code>webBuildId</code> and <code>coreBuildId</code> fields<br> <li> Update default values to "development" for build IDs<br> <li> Set placeholder timestamps for development builds</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-27336c978a24dc1efea27c315b22cea8a6ae27e9db6db4f69abd07faad604ad1">+4/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td> <details> <summary><strong>Sidebar.tsx</strong><dd><code>Display separate web and core build IDs in sidebar</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> web/src/components/Sidebar.tsx <ul><li>Add <code>BuildInfo</code> type with <code>webBuildId</code> and <code>coreBuildId</code> fields<br> <li> Update build info display to show separate web and core build IDs<br> <li> Simplify error handling in build info fetch logic<br> <li> Display build timestamp when available</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-219f9604c678f564c98ff1920201ea980331348dfb5dff7314a34d9fd198f1b1">+24/-18</a>&nbsp; </td> </tr> </table></td></tr><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>entrypoint-web.sh</strong><dd><code>Preserve build-info.json during public asset flattening</code>&nbsp; &nbsp; </dd></summary> <hr> docker/compose/entrypoint-web.sh <ul><li>Remove nested <code>build-info.json</code> before flattening public assets<br> <li> Copy root <code>build-info.json</code> back to nested location after flattening<br> <li> Apply same logic to both regular and standalone public directories</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-cbf60f692a5b524b141bf8b7fcff7abba11038d0b1c5a08378c0fd89e742d74e">+8/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-10-16 05:36:11 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1782#issuecomment-3409246145
Original created: 2025-10-16T05:36:11Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
JSON injection/breakage

Description: The genrule constructs JSON by string interpolation without escaping, which could break
JSON if VERSION or other inputs contain quotes or special characters; use a JSON-safe
generator or escaping to prevent malformed files.
BUILD.bazel [1032-1072]

Referred Code
set -euo pipefail

web_digest_file="$(location :web_image_base_amd64.digest)"
core_digest_file="$(location //docker/images:core_image_amd64.digest)"
version_file="$(location //:VERSION)"

web_digest=$$(cat "$$web_digest_file")
if [[ "$$web_digest" != sha256:* ]]; then
  echo "unexpected web digest format: $$web_digest" >&2
  exit 1
fi

web_short=$${web_digest#sha256:}
web_short=$$(printf '%s' "$$web_short" | cut -c1-12)

core_digest=$$(cat "$$core_digest_file")
if [[ "$$core_digest" != sha256:* ]]; then
  echo "unexpected core digest format: $$core_digest" >&2
  exit 1
fi



 ... (clipped 20 lines)
Ticket Compliance
🟡
🎫 #1776
🟢 Ensure build info/build ID changes appropriately for each Bazel build so that UI reflects
new builds.
Implement a reliable, reproducible build identifier mechanism suitable for Bazel-stamped
builds.
Surface the build identifiers in the application UI for verification.
Verify in a built container/image that build-info.json is correctly generated and
preserved after asset flattening at runtime.
Confirm UI shows distinct web and core build IDs and updates across successive Bazel
builds in deployed environments.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
- Requires Further Human Verification
🏷️ - Compliance label
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1782#issuecomment-3409246145 Original created: 2025-10-16T05:36:11Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/c1d21464436ecc651986bd1b3a19b43097f86941 --> Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td rowspan=1>⚪</td> <td><details><summary><strong>JSON injection/breakage </strong></summary><br> <b>Description:</b> The genrule constructs JSON by string interpolation without escaping, which could break <br>JSON if VERSION or other inputs contain quotes or special characters; use a JSON-safe <br>generator or escaping to prevent malformed files.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1782/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80R1032-R1072'>BUILD.bazel [1032-1072]</a></strong><br> <details open><summary>Referred Code</summary> ```txt set -euo pipefail web_digest_file="$(location :web_image_base_amd64.digest)" core_digest_file="$(location //docker/images:core_image_amd64.digest)" version_file="$(location //:VERSION)" web_digest=$$(cat "$$web_digest_file") if [[ "$$web_digest" != sha256:* ]]; then echo "unexpected web digest format: $$web_digest" >&2 exit 1 fi web_short=$${web_digest#sha256:} web_short=$$(printf '%s' "$$web_short" | cut -c1-12) core_digest=$$(cat "$$core_digest_file") if [[ "$$core_digest" != sha256:* ]]; then echo "unexpected core digest format: $$core_digest" >&2 exit 1 fi ... (clipped 20 lines) ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/1776>#1776</a></summary> <table width='100%'><tbody> <tr><td rowspan=3>🟢</td> <td>Ensure build info/build ID changes appropriately for each Bazel build so that UI reflects <br>new builds.</td></tr> <tr><td>Implement a reliable, reproducible build identifier mechanism suitable for Bazel-stamped <br>builds.</td></tr> <tr><td>Surface the build identifiers in the application UI for verification.</td></tr> <tr><td rowspan=2>⚪</td> <td>Verify in a built container/image that build-info.json is correctly generated and <br>preserved after asset flattening at runtime.</td></tr> <tr><td>Confirm UI shows distinct web and core build IDs and updates across successive Bazel <br>builds in deployed environments.</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>No custom compliance provided</strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </details></td></tr> <tr><td align="center" colspan="2"> <!-- placeholder --> <!-- /compliance --update_compliance=true --> </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>
qodo-code-review[bot] commented 2025-10-16 05:37:10 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1782#issuecomment-3409247817
Original created: 2025-10-16T05:37:10Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Simplify the complex build process

The build process has become overly complex by using four tar layers for one
file and an entrypoint script for file manipulation. This should be simplified
by adjusting the Bazel build rules to create the correct directory structure
directly, which would improve robustness and maintainability.

Examples:

docker/images/BUILD.bazel [1075-1128]
pkg_tar(
    name = "web_build_info_root_layer_amd64",
    files = {
        ":web_build_info_json": "app/public/build-info.json",
    },
    modes = {
        "app/public/build-info.json": "0644",
    },
    package_dir = "/",
)

 ... (clipped 44 lines)
docker/compose/entrypoint-web.sh [135-151]
    rm -f "${NESTED_PUBLIC}/build-info.json"
    cp -R "${NESTED_PUBLIC}/." "${PUBLIC_ROOT}/"
    if [ -f "${PUBLIC_ROOT}/build-info.json" ]; then
        cp "${PUBLIC_ROOT}/build-info.json" "${NESTED_PUBLIC}/build-info.json"
    fi
fi

STANDALONE_PUBLIC_ROOT="/app/.next/standalone/public"
NESTED_STANDALONE_PUBLIC="${STANDALONE_PUBLIC_ROOT}/web/public_flat"
if [ -d "${NESTED_STANDALONE_PUBLIC}" ]; then

 ... (clipped 7 lines)

Solution Walkthrough:

Before:

# BUILD.bazel
# 1. Generate a single build-info.json file
genrule(name = "web_build_info_json", ...)

# 2. Create 4 separate tar layers to place the same file
#    in 4 different locations inside the image.
pkg_tar(name = "layer1", files = {":web_build_info_json": "path/one/build-info.json"})
pkg_tar(name = "layer2", files = {":web_build_info_json": "path/two/build-info.json"})
# ... (2 more layers)

# entrypoint-web.sh
# 3. At container startup, shuffle files around to fix the structure.
rm -f "path/two/build-info.json"
cp -R "path/two/." "path/one/"
cp "path/one/build-info.json" "path/two/build-info.json"

After:

# BUILD.bazel
# 1. Generate a single build-info.json file
genrule(name = "web_build_info_json", ...)

# 2. Use a single, more sophisticated rule (e.g., a single pkg_tar
#    or a custom genrule) to create a single layer with the
#    correct file structure from the start.
pkg_tar(
    name = "web_assets_layer",
    files = {
        ":web_build_info_json": "path/one/build-info.json",
        # Potentially include other assets here
    },
    # ... other properties to create the desired structure
)

# entrypoint-web.sh
# 3. No file shuffling is needed at runtime.
# (script is simplified or file manipulation logic is removed)

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant increase in complexity in the build process, involving four redundant pkg_tar layers and runtime file manipulation, and proposes a more robust and maintainable architectural solution.

High
General
Use async/await for data fetching

Refactor the fetch call within React.useEffect to use async/await syntax instead
of .then() and .catch() for improved readability and error handling.

web/src/components/Sidebar.tsx [52-57]

 React.useEffect(() => {
-    // Try to fetch build info from public file
-    fetch('/build-info.json')
-        .then(res => (res.ok ? res.json() : Promise.reject()))
-        .then((data: BuildInfo) => setBuildInfo(data))
-        .catch(() => setBuildInfo({}));
+    const fetchBuildInfo = async () => {
+        try {
+            const res = await fetch('/build-info.json');
+            if (res.ok) {
+                const data: BuildInfo = await res.json();
+                setBuildInfo(data);
+            } else {
+                setBuildInfo({});
+            }
+        } catch (error) {
+            console.error('Failed to fetch build info:', error);
+            setBuildInfo({});
+        }
+    };
+
+    fetchBuildInfo();
 }, []);
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion refactors the data fetching logic to use async/await, which improves readability and aligns with modern JavaScript practices.

Low
Simplify SHA hash truncation logic

Simplify the two-step SHA hash truncation into a single line by piping the
output of the parameter expansion directly to cut.

docker/images/BUILD.bazel [1044-1045]

-web_short=$${web_digest#sha256:}
-web_short=$$(printf '%s' "$$web_short" | cut -c1-12)
+web_short=$$(echo -n "$${web_digest#sha256:}" | cut -c1-12)
  • Apply / Chat
Suggestion importance[1-10]: 3

__

Why: The suggestion correctly combines two lines into one for conciseness, but the improvement is minor and replaces the more robust printf with echo.

Low
  • More
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1782#issuecomment-3409247817 Original created: 2025-10-16T05:37:10Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## PR Code Suggestions ✨ <!-- c1d2146 --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Simplify the complex build process</summary> ___ **The build process has become overly complex by using four tar layers for one <br>file and an entrypoint script for file manipulation. This should be simplified <br>by adjusting the Bazel build rules to create the correct directory structure <br>directly, which would improve robustness and maintainability.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80R1075-R1128">docker/images/BUILD.bazel [1075-1128]</a> </summary> ```starlark pkg_tar( name = "web_build_info_root_layer_amd64", files = { ":web_build_info_json": "app/public/build-info.json", }, modes = { "app/public/build-info.json": "0644", }, package_dir = "/", ) ... (clipped 44 lines) ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1782/files#diff-cbf60f692a5b524b141bf8b7fcff7abba11038d0b1c5a08378c0fd89e742d74eR135-R151">docker/compose/entrypoint-web.sh [135-151]</a> </summary> ```bash rm -f "${NESTED_PUBLIC}/build-info.json" cp -R "${NESTED_PUBLIC}/." "${PUBLIC_ROOT}/" if [ -f "${PUBLIC_ROOT}/build-info.json" ]; then cp "${PUBLIC_ROOT}/build-info.json" "${NESTED_PUBLIC}/build-info.json" fi fi STANDALONE_PUBLIC_ROOT="/app/.next/standalone/public" NESTED_STANDALONE_PUBLIC="${STANDALONE_PUBLIC_ROOT}/web/public_flat" if [ -d "${NESTED_STANDALONE_PUBLIC}" ]; then ... (clipped 7 lines) ``` </details> ### Solution Walkthrough: #### Before: ```bash # BUILD.bazel # 1. Generate a single build-info.json file genrule(name = "web_build_info_json", ...) # 2. Create 4 separate tar layers to place the same file # in 4 different locations inside the image. pkg_tar(name = "layer1", files = {":web_build_info_json": "path/one/build-info.json"}) pkg_tar(name = "layer2", files = {":web_build_info_json": "path/two/build-info.json"}) # ... (2 more layers) # entrypoint-web.sh # 3. At container startup, shuffle files around to fix the structure. rm -f "path/two/build-info.json" cp -R "path/two/." "path/one/" cp "path/one/build-info.json" "path/two/build-info.json" ``` #### After: ```bash # BUILD.bazel # 1. Generate a single build-info.json file genrule(name = "web_build_info_json", ...) # 2. Use a single, more sophisticated rule (e.g., a single pkg_tar # or a custom genrule) to create a single layer with the # correct file structure from the start. pkg_tar( name = "web_assets_layer", files = { ":web_build_info_json": "path/one/build-info.json", # Potentially include other assets here }, # ... other properties to create the desired structure ) # entrypoint-web.sh # 3. No file shuffling is needed at runtime. # (script is simplified or file manipulation logic is removed) ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a significant increase in complexity in the build process, involving four redundant `pkg_tar` layers and runtime file manipulation, and proposes a more robust and maintainable architectural solution. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Use async/await for data fetching</summary> ___ **Refactor the <code>fetch</code> call within <code>React.useEffect</code> to use <code>async/await</code> syntax instead <br>of <code>.then()</code> and <code>.catch()</code> for improved readability and error handling.** [web/src/components/Sidebar.tsx [52-57]](https://github.com/carverauto/serviceradar/pull/1782/files#diff-219f9604c678f564c98ff1920201ea980331348dfb5dff7314a34d9fd198f1b1R52-R57) ```diff React.useEffect(() => { - // Try to fetch build info from public file - fetch('/build-info.json') - .then(res => (res.ok ? res.json() : Promise.reject())) - .then((data: BuildInfo) => setBuildInfo(data)) - .catch(() => setBuildInfo({})); + const fetchBuildInfo = async () => { + try { + const res = await fetch('/build-info.json'); + if (res.ok) { + const data: BuildInfo = await res.json(); + setBuildInfo(data); + } else { + setBuildInfo({}); + } + } catch (error) { + console.error('Failed to fetch build info:', error); + setBuildInfo({}); + } + }; + + fetchBuildInfo(); }, []); ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion refactors the data fetching logic to use `async/await`, which improves readability and aligns with modern JavaScript practices. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Simplify SHA hash truncation logic</summary> ___ **Simplify the two-step SHA hash truncation into a single line by piping the <br>output of the parameter expansion directly to <code>cut</code>.** [docker/images/BUILD.bazel [1044-1045]](https://github.com/carverauto/serviceradar/pull/1782/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80R1044-R1045) ```diff -web_short=$${web_digest#sha256:} -web_short=$$(printf '%s' "$$web_short" | cut -c1-12) +web_short=$$(echo -n "$${web_digest#sha256:}" | cut -c1-12) ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 3</summary> __ Why: The suggestion correctly combines two lines into one for conciseness, but the improvement is minor and replaces the more robust `printf` with `echo`. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More <!-- /improve --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2326
No description provided.