chore(docs): update notice file#171
Conversation
521a73e to
e85bd40
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughAdds a third-party logos notice link to root notice generation, updates notice and favicon content, changes site HTML minification and SSR allowlists, and adds a context7 JSON config. ChangesDocumentation Logos Notice and Favicon Update
Site Minification and SSR Allowlist Updates
Context7 configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
projects/site/src/_11ty/layouts/metadata.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. projects/site/src/_11ty/transforms/html-minify.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
e85bd40 to
8c3bd93
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/site/public/static/images/integrations/NOTICE.md`:
- Line 9: The NOTICE table in the static images integrations list is no longer
alphabetically ordered because the logo.svg entry is placed first. Reorder the
row in the table so logo.svg appears in its correct alphabetical position
alongside the other filenames, using the existing NOTICE.md table structure as
the reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b919d7b4-43d6-4eec-b44c-e904985c49ef
⛔ Files ignored due to path filters (1)
projects/site/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
.vale.iniNOTICE.mdprojects/internals/ci/notice/index.jsprojects/internals/ci/notice/template.jsprojects/site/public/static/images/integrations/NOTICE.mdprojects/site/src/_11ty/layouts/common.js
8c3bd93 to
34465ae
Compare
- add missing site ico - add additional notice details for documentation usage of third party logos Signed-off-by: Cory Rylan <crylan@nvidia.com>
34465ae to
aea5b9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/site/src/_11ty/layouts/common.js`:
- Line 46: The favicon in the common layout is hotlinking an external GitHub
avatar URL, so replace the hardcoded href in the layout’s favicon link with a
locally served static asset. Update the `common.js` template to point to the
site’s own favicon file and keep the `<link rel="icon">` entry using a stable
self-hosted path, so page loads no longer depend on the external avatar service.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 584a5520-084c-416c-a407-e3317ec1317d
⛔ Files ignored due to path filters (1)
projects/site/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
.vale.iniNOTICE.mdcontext7.jsonprojects/internals/ci/notice/index.jsprojects/internals/ci/notice/template.jsprojects/site/public/static/images/integrations/NOTICE.mdprojects/site/src/_11ty/layouts/common.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/site/src/_11ty/transforms/html-minify.js`:
- Around line 8-13: The html-minify transform is swallowing minification
failures without any diagnostic, so update the try/catch around minifyHTML in
the transform function to log the caught error before falling back to the
original content. Keep the existing fallback behavior for watch/build safety,
but make sure the catch path includes a clear error message and the actual
exception details so failures are visible in build logs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 994c8a2a-ab71-453b-b80b-c38d514f2bbc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
projects/site/eleventy.config.jsprojects/site/package.jsonprojects/site/src/_11ty/transforms/html-minify.js
💤 Files with no reviewable changes (2)
- projects/site/eleventy.config.js
- projects/site/package.json
| try { | ||
| result = process.env.ELEVENTY_RUN_MODE !== 'watch' ? minifyStyleTags(minifyHTML(content)) : content; | ||
| result = process.env.ELEVENTY_RUN_MODE !== 'watch' ? await minifyHTML(content) : content; | ||
| } catch (_) { | ||
| result = content; | ||
| } | ||
| return result; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Silent error swallowing hides minification failures.
The catch (_) discards the error and falls back to unminified content without any diagnostic. In build mode a minification failure would then silently ship larger, unminified HTML with no signal in the build logs. Consider logging the error before falling back.
♻️ Proposed change
try {
result = process.env.ELEVENTY_RUN_MODE !== 'watch' ? await minifyHTML(content) : content;
- } catch (_) {
+ } catch (error) {
+ console.warn('html-minify: minification failed, using unminified content', error);
result = content;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| result = process.env.ELEVENTY_RUN_MODE !== 'watch' ? minifyStyleTags(minifyHTML(content)) : content; | |
| result = process.env.ELEVENTY_RUN_MODE !== 'watch' ? await minifyHTML(content) : content; | |
| } catch (_) { | |
| result = content; | |
| } | |
| return result; | |
| try { | |
| result = process.env.ELEVENTY_RUN_MODE !== 'watch' ? await minifyHTML(content) : content; | |
| } catch (error) { | |
| console.warn('html-minify: minification failed, using unminified content', error); | |
| result = content; | |
| } | |
| return result; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/_11ty/transforms/html-minify.js` around lines 8 - 13, The
html-minify transform is swallowing minification failures without any
diagnostic, so update the try/catch around minifyHTML in the transform function
to log the caught error before falling back to the original content. Keep the
existing fallback behavior for watch/build safety, but make sure the catch path
includes a clear error message and the actual exception details so failures are
visible in build logs.
- fixes issue with style tag minification - removes direct dependency for css minification Signed-off-by: Cory Rylan <crylan@nvidia.com>
ac66bc9 to
f7c2717
Compare
Summary by CodeRabbit