[MG_A1_00]: Initial-base-setup - #1
Conversation
📝 WalkthroughWalkthroughThe PR adds Sass design tokens, reusable utilities, resets, themes, components, icon-font data, structured public content, and WebP assets. It also updates the HTML stylesheet path and reformats two YAML files without changing their semantics. ChangesFrontend setup and styling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bd6b713e-ba36-46bf-b06d-169e7ba6828f
⛔ Files ignored due to path filters (22)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.jsonpublic/assets/fonts/icomoon.eotis excluded by!**/*.eotpublic/assets/fonts/icomoon.svgis excluded by!**/*.svgpublic/assets/fonts/icomoon.ttfis excluded by!**/*.ttfpublic/assets/fonts/icomoon.woffis excluded by!**/*.woffpublic/assets/images/Group 3.svgis excluded by!**/*.svgpublic/assets/images/airbnb.pngis excluded by!**/*.pngpublic/assets/images/booking.pngis excluded by!**/*.pngpublic/assets/images/carousel.pngis excluded by!**/*.pngpublic/assets/images/city.jpgis excluded by!**/*.jpgpublic/assets/images/expedia.pngis excluded by!**/*.pngpublic/assets/images/globe.pngis excluded by!**/*.pngpublic/assets/images/logo.svgis excluded by!**/*.svgpublic/assets/images/mountain.jpgis excluded by!**/*.jpgpublic/assets/images/object.svgis excluded by!**/*.svgpublic/assets/images/ocean.jpgis excluded by!**/*.jpgpublic/assets/images/orbitz.pngis excluded by!**/*.pngpublic/assets/images/star.svgis excluded by!**/*.svgpublic/assets/images/travel-point.pngis excluded by!**/*.pngpublic/assets/images/triangles.pngis excluded by!**/*.pngpublic/assets/images/tripadvisor.pngis excluded by!**/*.pngpublic/assets/images/user.jpgis excluded by!**/*.jpg
📒 Files selected for processing (20)
.coderabbit.yaml.github/workflows/pr-check.ymlindex.htmlpackage.jsonpublic/assets/fonts/selection.jsonpublic/content.jsonpublic/styles/main.csssrc/styles/main.scsssrc/styles/scss/abstract/_breakpoint.scsssrc/styles/scss/abstract/_color.scsssrc/styles/scss/abstract/_function.scsssrc/styles/scss/abstract/_mixin.scsssrc/styles/scss/base/_reset.scsssrc/styles/scss/base/_typography.scsssrc/styles/scss/components/_button.scsssrc/styles/scss/components/_cards.scsssrc/styles/scss/components/_floater.scsssrc/styles/scss/components/_icon.scsssrc/styles/scss/themes/_light.scsssrc/styles/scss/vendors/_icomoon.scss
💤 Files with no reviewable changes (1)
- src/styles/main.scss
| - name: Checkout (full history for commit range) | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file =="
cat -n .github/workflows/pr-check.yml | sed -n '1,80p'
echo
echo "== all checkout refs in workflows =="
rg -n "actions/checkout|persist-credentials|fetch-depth" .github/workflows || true
echo
echo "== diff stat/name-status =="
git diff --stat || true
git diff --name-status || trueRepository: jtg-inductions-fe/FE-Assignment-1-Manas-Gulati-UID00782-2026
Length of output: 3868
🌐 Web query:
actions/checkout v4 persist-credentials default true token local git config README
💡 Result:
In actions/checkout v4, the persist-credentials input has a default value of true [1][2][3]. When set to true, actions/checkout configures the repository's local git configuration to include an authentication token (typically the GITHUB_TOKEN) [4][5][6]. This is implemented by adding an http.{host}.extraheader configuration to the local.git/config file, which includes the authorization information [5][6]. This mechanism allows subsequent steps in the same job to execute authenticated git commands without needing to manually re-configure credentials [4][3][6]. If you set persist-credentials to false, this configuration step is skipped, and no authentication information is persisted in the local git config [5][6]. This is often recommended as a security best practice to reduce the risk of credential leakage if the workspace (including the.git directory) is uploaded as an artifact or accessed by malicious processes [6]. Note that setting this to false will prevent downstream git commands from using the automatically configured token, which may interfere with tools like gh auth setup-git that expect to manage their own credentials [5].
Citations:
- 1: https://github.com/actions/checkout/blob/v4/action.yml
- 2: https://github.com/actions/checkout/blob/v4.1.1/action.yml
- 3: https://github.com/marketplace/actions/checkout
- 4: https://github.com/actions/checkout/blob/v4/README.md
- 5:
gh auth setup-gitdoes not override the git user following anactions/checkoutinvocation cli/cli#10905 - 6: Set
persist-credentials: falseon checkout steps cli/go-gh#225
Pin checkout to a commit SHA and disable credential persistence.
actions/checkout@v4 is tag-pinned, so the target can change. Also, checkout persists the workflow token in local Git config by default, which leaves credentials available to later steps. Pin a reviewed full SHA and add persist-credentials: false if the workflow only needs read-only checkout access.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 15-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/styles/scss/abstract/_function.scss (1)
3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign doc-comment style with the rest of the codebase.
This uses a manual
/*@name/@param/@return */block, while_mixin.scssuses SassDoc///triple-slash comments. SassDoc tooling only parses///comments, so this block won't be picked up consistently. As per path instructions, ensure "proper SCSS mixins usage and standard SASS documentation/comments."Source: Path instructions
src/styles/scss/vendors/_icomoon.scss (1)
17-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFont-family value still quoted — Stylelint fix only partially applied.
Line 2 was already fixed to drop the quotes around
icomoon, but Line 17 still has'icomoon', which will re-trigger the samefont-family-name-quoteslint failure previously flagged.Source: Linters/SAST tools
♻️ Duplicate comments (2)
src/styles/scss/base/_typography.scss (2)
1-7: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAddress the
//need to import fontsTODO.
$font-family-primary('Circular Std') and$font-family-secondary(Inter) are referenced throughout the typography maps, but no@font-face/font-loading exists yet, so they'll render with fallback fonts until this is wired up.
Want me to draft the@font-facedeclarations or open a tracking issue for this?
26-85: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTypography presets use a single fixed
font-sizewith no breakpoint variation.None of
$main-heading-*,$sub-heading-*,$subtitle-1, or$nav-link-*vary by screen size, so headings will render identically on mobile and desktop despitemixin.responsiveUpexisting specifically to support size-adaptive behavior. Consider expressing per-breakpoint overrides (e.g., via nested maps orresponsiveUpcalls in the consuming component) so heading scale actually adapts across breakpoints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b40ad890-8582-4b30-8073-4cca9e6f862b
📒 Files selected for processing (27)
public/assets/images/Ellipse 23.webppublic/assets/images/Group 3.webppublic/assets/images/airbnb.webppublic/assets/images/booking.webppublic/assets/images/carousel.webppublic/assets/images/city.webppublic/assets/images/expedia.webppublic/assets/images/globe.webppublic/assets/images/logo.webppublic/assets/images/mountain.webppublic/assets/images/object.webppublic/assets/images/ocean.webppublic/assets/images/orbitz.webppublic/assets/images/plane.webppublic/assets/images/star.webppublic/assets/images/travel-point.webppublic/assets/images/triangles.webppublic/assets/images/tripadvisor.webppublic/assets/images/user.webpsrc/Assets/Fonts/Icomoon backup/selection.jsonsrc/styles/scss/abstract/_breakpoint.scsssrc/styles/scss/abstract/_function.scsssrc/styles/scss/abstract/_mixin.scsssrc/styles/scss/base/_typography.scsssrc/styles/scss/components/_cards.scsssrc/styles/scss/components/_chip.scsssrc/styles/scss/vendors/_icomoon.scss
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/styles/scss/abstract/_function.scss (2)
3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse valid SassDoc for
px-to-rem.The parameter and return annotations use malformed spacing and nonstandard formatting, so the helper contract is not reliably documented or parsed.
As per path instructions, SCSS files must use proper SCSS mixins and standard SASS documentation/comments.
Source: Path instructions
11-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalize or constrain the
$baseunit.The function strips units from
$pixelbut not$base. Since the documentation references16px, passing a unit-bearing base can produce an invalid compoundrem/pxresult and fail Sass compilation. Keep the base contract consistent with the implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ae01a2a-5f5b-4137-8aad-e66de85e4546
📒 Files selected for processing (7)
src/styles/scss/abstract/_function.scsssrc/styles/scss/abstract/_mixin.scsssrc/styles/scss/base/_typography.scsssrc/styles/scss/components/_button.scsssrc/styles/scss/components/_cards.scsssrc/styles/scss/components/_icon.scsssrc/styles/scss/themes/_light.scss
💤 Files with no reviewable changes (1)
- src/styles/scss/base/_typography.scss
9128371 to
419e960
Compare
2 participants
Summary by CodeRabbit