fix: hash generated files #15253
fix: hash generated files #15253
Conversation
lodmfjord
added
deploy-feature
WalkthroughRecent modifications involve the refactor of hash generation for caching scripts in the CI environment. The singular file hash function ( Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
scripts/ci/cache/_utils.mjs (1)
Line range hint
33-33: Optimize the use of spread syntax ingetPackageHash.The use of the spread syntax in the
reducefunction can lead to performance issues due to itsO(n^2)complexity in this context. Consider using a more efficient method such as.pushor directly modifying the accumulator.export async function getPackageHash( root = ROOT, keys = ['resolutions', 'dependencies', 'devDependencies'], ) { const content = await getPackageJSON(root); const value = keys.reduce((a, b) => { a[b] = content[b]; return a; }, {}); return crypto.createHash('sha256').update(JSON.stringify(value)).digest('hex'); }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
scripts/ci/cache/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (5)
- scripts/ci/cache/__config.mjs (3 hunks)
- scripts/ci/cache/_const.mjs (1 hunks)
- scripts/ci/cache/_generated_files.mjs (1 hunks)
- scripts/ci/cache/_utils.mjs (1 hunks)
- scripts/ci/cache/package.json (1 hunks)
Files skipped from review due to trivial changes (2)
- scripts/ci/cache/_const.mjs
- scripts/ci/cache/package.json
Additional context used
Path-based instructions (3)
scripts/ci/cache/_generated_files.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
scripts/ci/cache/__config.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
scripts/ci/cache/_utils.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
Biome
scripts/ci/cache/_utils.mjs
[error] 33-33: Avoid the use of spread (
...) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.
Additional comments not posted (2)
scripts/ci/cache/__config.mjs (2)
22-22: Integration ofgetGeneratedFilesHashseems correct.The addition of
getGeneratedFilesHashfrom './_generated_files.mjs' is correctly imported and used in this file. This change aligns with the PR's objective to manage hashing of generated files more effectively.
88-88: Validate the integration and usage of multiple asynchronous operations.Given that multiple asynchronous hash computations and other operations are involved in constructing the cache key, it's critical to ensure that they are correctly awaited and integrated. This is crucial for maintaining the integrity and correctness of the cache mechanism.
Verification successful
The functions
getYarnLockHash,getPackageHash,getGeneratedFilesHash, andrunCommandare all defined as asynchronous in their respective files. This confirms that they are correctly awaited in thescripts/ci/cache/__config.mjsfile. The integration of these asynchronous operations in constructing the cache key is thus maintained correctly.
getYarnLockHash: Defined asasyncinscripts/ci/get-node-modules-hash.mjsandscripts/ci/cache/_utils.mjs.getPackageHash: Defined asasyncinscripts/ci/get-node-modules-hash.mjsandscripts/ci/cache/_utils.mjs.getGeneratedFilesHash: Defined asasyncinscripts/ci/cache/_generated_files.mjs.runCommand: Defined asasyncinscripts/ci/cache/_utils.mjs.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify correct integration and usage of asynchronous operations in cache configuration. # Test: Check for proper usage of `await` with asynchronous functions. rg --type python $'await'Length of output: 55
Script:
#!/bin/bash # Description: Verify correct integration and usage of asynchronous operations in cache configuration. # Test: Check for proper usage of `await` with asynchronous functions in the specified JavaScript file. rg 'await' scripts/ci/cache/__config.mjsLength of output: 1287
Script:
#!/bin/bash # Description: Verify that functions used with `await` are asynchronous. # Find definitions of functions used with `await` in the specified JavaScript file. ast-grep --lang javascript --pattern 'async function $_($$$) { $$$ }' scripts/ci/cache/__config.mjsLength of output: 101
Script:
#!/bin/bash # Description: Verify that functions used with `await` are asynchronous. # Find definitions of functions used with `await`, including async arrow functions and imports. ast-grep --lang javascript --pattern 'const $_ = async ($$$) => { $$$ }' scripts/ci/cache/__config.mjs ast-grep --lang javascript --pattern 'import { $_ } from $$$' scripts/ci/cache/__config.mjsLength of output: 543
Script:
#!/bin/bash # Description: Verify that functions used with `await` are asynchronous. # Find all function definitions in the specified JavaScript file. ast-grep --lang javascript --pattern 'function $_($$$) { $$$ }' scripts/ci/cache/__config.mjs ast-grep --lang javascript --pattern 'const $_ = ($$$) => { $$$ }' scripts/ci/cache/__config.mjs ast-grep --lang javascript --pattern 'import $_ from $$$' scripts/ci/cache/__config.mjs ast-grep --lang javascript --pattern 'import { $$$ } from $$$' scripts/ci/cache/__config.mjsLength of output: 1069
Script:
#!/bin/bash # Description: Verify that functions used with `await` are asynchronous. # Find definitions of functions used with `await` in the entire codebase. rg 'function getYarnLockHash' --files-with-matches rg 'function getPackageHash' --files-with-matches rg 'function getGeneratedFilesHash' --files-with-matches rg 'function runCommand' --files-with-matches # Find imports of these functions in the entire codebase. rg 'import { getYarnLockHash' --files-with-matches rg 'import { getPackageHash' --files-with-matches rg 'import { getGeneratedFilesHash' --files-with-matches rg 'import { runCommand' --files-with-matchesLength of output: 1103
Script:
#!/bin/bash # Description: Verify the asynchronous nature of functions used with `await`. # Check definitions of functions in the identified files. ast-grep --lang javascript --pattern 'async function getYarnLockHash($$$) { $$$ }' scripts/ci/get-node-modules-hash.mjs scripts/ci/cache/_utils.mjs ast-grep --lang javascript --pattern 'async function getPackageHash($$$) { $$$ }' scripts/ci/get-node-modules-hash.mjs scripts/ci/cache/_utils.mjs ast-grep --lang javascript --pattern 'async function getGeneratedFilesHash($$$) { $$$ }' scripts/ci/cache/_generated_files.mjs ast-grep --lang javascript --pattern 'async function runCommand($$$) { $$$ }' scripts/ci/cache/_utils.mjs infra/src/common.tsLength of output: 6785
|
Affected services are: air-discount-scheme-api,air-discount-scheme-backend,api,application-system-api,download-service,financial-aid-api,financial-aid-backend,financial-aid-open-api,github-actions-cache,icelandic-names-registry-backend,judicial-system-api,judicial-system-digital-mailbox-api,judicial-system-backend,judicial-system-message-handler,judicial-system-robot-api,judicial-system-scheduler,judicial-system-xrd-api,license-api,reference-backend,regulations-admin-backend,services-auth-admin-api,services-auth-delegation-api,services-auth-ids-api,services-auth-personal-representative,services-auth-personal-representative-public,services-auth-public-api,services-contentful-entry-tagger,services-documents,services-endorsements-api,services-search-indexer,services-sessions,services-university-gateway,services-user-notification,services-user-profile,services-xroad-collector,skilavottord-ws,air-discount-scheme-web,auth-admin-web,consultation-portal,contentful-apps,financial-aid-web-osk,financial-aid-web-veita,judicial-system-web,skilavottord-web,web,application-system-form,island-ui-storybook,portals-admin,service-portal,system-e2e,external-contracts-tests, |
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 99 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15253 +/- ##
=======================================
Coverage 37.10% 37.10%
=======================================
Files 6448 6448
Lines 131455 131455
Branches 37574 37574
=======================================
Hits 48781 48781
Misses 82674 82674
Flags with carried forward coverage won't be shown. Click here to find out more. see 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
scripts/ci/cache/_utils.mjs (3)
Line range hint
32-32: Optimize thereduceoperation to avoid performance issues.- const value = keys.reduce((a, b) => { - return { - ...a, - [b]: content[b], - } - }, {}) + const value = {}; + for (const key of keys) { + value[key] = content[key]; + }This change avoids the performance issue with using spread syntax in a reduce operation, which has a time complexity of
O(n^2). The proposed loop has a linear time complexity.
Line range hint
8-17: Ensure robust error handling by checking for nullcontent.+ if (!content) { + throw new Error('package.json is not available or malformed'); + } const nodeVersion = content?.engines?.node; const yarnVersion = content?.engines?.yarn;Adding a null check for
contentbefore accessing properties will prevent potential runtime errors if thepackage.jsonfile is missing or malformed.
Line range hint
160-192: Enhance error handling inrunCommandto reject the promise with a meaningful error message.- // reject(`Error: ${error.message}`) + reject(`Error: ${error.message}`)Uncommenting the rejection line ensures that the promise returned by
runCommandproperly handles errors by rejecting with a meaningful error message.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- scripts/ci/cache/_generated_files.mjs (1 hunks)
- scripts/ci/cache/_utils.mjs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- scripts/ci/cache/_generated_files.mjs
Additional context used
Path-based instructions (1)
scripts/ci/cache/_utils.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
Biome
scripts/ci/cache/_utils.mjs
[error] 32-32: Avoid the use of spread (
...) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.
Additional comments not posted (1)
scripts/ci/cache/_utils.mjs (1)
Line range hint
19-20: The implementation ofgetPlatformStringis straightforward and correct.
* fix: hash generated files * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is>
…15239) * Fix locale updates. Add cleanstring * update string clean * Update packagejson * Update query * Update query * Add lang option * Test * Fix * Add locale * Test name update * fix: force generated files * chore: nx format:write update dirty files * Revert test * Remove ff * fix: hash generated files * fix(j-s): Null Service Reference (#15247) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(native-app): Notifications final improvements (#15240) * feat: buttons at top should not be sticky * feat: add empty state for notifications * fix: make sure to update unreadCount in inbox when fetching next page * feat: add fallback icon for notifications --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(consultation-portal): KAM-2522: PowerBI report added (#15251) * fix: hash generated files (#15253) * fix: hash generated files * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> * fix: remove file * Update __config.mjs --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: lommi <jonorn@gmail.com> Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Guðjón Guðjónsson <gudjon@kolibri.is> Co-authored-by: Þórey Jóna <thoreyjona@gmail.com> Co-authored-by: Karl Arnar Ægisson <korsbacka@gmail.com>
…15239) * Fix locale updates. Add cleanstring * update string clean * Update packagejson * Update query * Update query * Add lang option * Test * Fix * Add locale * Test name update * fix: force generated files * chore: nx format:write update dirty files * Revert test * Remove ff * fix: hash generated files * fix(j-s): Null Service Reference (#15247) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(native-app): Notifications final improvements (#15240) * feat: buttons at top should not be sticky * feat: add empty state for notifications * fix: make sure to update unreadCount in inbox when fetching next page * feat: add fallback icon for notifications --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(consultation-portal): KAM-2522: PowerBI report added (#15251) * fix: hash generated files (#15253) * fix: hash generated files * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> * fix: remove file * Update __config.mjs --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: lommi <jonorn@gmail.com> Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Guðjón Guðjónsson <gudjon@kolibri.is> Co-authored-by: Þórey Jóna <thoreyjona@gmail.com> Co-authored-by: Karl Arnar Ægisson <korsbacka@gmail.com>
…15255) * fix(documents-v2): Add download URL to detail document v2 (#15194) * fix(service-portal): Update notification icon styling (#15219) * Update notification icon styling * Add option to remove button docline * fix(service-portal): Notifications - locale updates. Add cleanstring (#15239) * Fix locale updates. Add cleanstring * update string clean * Update packagejson * Update query * Update query * Add lang option * Test * Fix * Add locale * Test name update * fix: force generated files * chore: nx format:write update dirty files * Revert test * Remove ff * fix: hash generated files * fix(j-s): Null Service Reference (#15247) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(native-app): Notifications final improvements (#15240) * feat: buttons at top should not be sticky * feat: add empty state for notifications * fix: make sure to update unreadCount in inbox when fetching next page * feat: add fallback icon for notifications --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(consultation-portal): KAM-2522: PowerBI report added (#15251) * fix: hash generated files (#15253) * fix: hash generated files * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> * fix: remove file * Update __config.mjs --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: lommi <jonorn@gmail.com> Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Guðjón Guðjónsson <gudjon@kolibri.is> Co-authored-by: Þórey Jóna <thoreyjona@gmail.com> Co-authored-by: Karl Arnar Ægisson <korsbacka@gmail.com> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: lommi <jonorn@gmail.com> Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Guðjón Guðjónsson <gudjon@kolibri.is> Co-authored-by: Þórey Jóna <thoreyjona@gmail.com> Co-authored-by: Karl Arnar Ægisson <korsbacka@gmail.com>
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
Refactor
getGeneratedFilesHashand removinggetGeneratedFileHash.Chores