-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Launch Split Up Git Commands For Stage 1 experiment #2544
Conversation
WalkthroughThis pull request introduces modifications to the course stage tutorial components, focusing on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Bundle ReportChanges will increase total bundle size by 36.06MB (100.0%) ⬆️
|
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky tests
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
app/components/course-page/course-stage-step/first-stage-tutorial-card/submit-code-step.ts (1)
29-31
: Consider removing unnecessary getter.The
currentCourse
getter simply forwardscourseStage.course
. Consider usingthis.args.courseStage.course
directly where needed, unless this getter serves a specific purpose not apparent from the context.app/components/course-page/course-stage-step/first-stage-tutorial-card/submit-code-step.hbs (5)
8-8
: Consider providing more guidance on commit messages.The example
[any message]
might be too vague for beginners. Consider providing a more meaningful example or explaining commit message best practices.- <CopyableTerminalCommand @commands={{array 'git commit -am "[any message]"'}} class="mb-3" /> + <CopyableTerminalCommand @commands={{array 'git commit -am "implement stage 1 solution"'}} class="mb-3" />
15-15
: Add more context to the example output.The example output shows numbers that might vary for users. Consider adding a note explaining that the specific numbers and hash may differ.
126-126
: Avoid hardcoding avatar URLs.The avatar URL should be provided through a configuration or component property rather than being hardcoded. This makes it easier to update and maintain.
171-171
: Extract forum URL template to a constant.The forum URL template with query parameters is duplicated. Consider extracting it to a constant or helper function for better maintainability.
Also applies to: 118-118
83-84
: Enhance modal accessibility.Consider adding:
aria-label
for the modal backdroprole="dialog"
andaria-labelledby
attributes for the modal body- Keyboard focus trap within the modal
Also applies to: 136-137
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs
(1 hunks)app/components/course-page/course-stage-step/first-stage-tutorial-card.ts
(1 hunks)app/components/course-page/course-stage-step/first-stage-tutorial-card/submit-code-step.hbs
(2 hunks)app/components/course-page/course-stage-step/first-stage-tutorial-card/submit-code-step.ts
(1 hunks)app/components/course-page/course-stage-step/second-stage-tutorial-card.ts
(0 hunks)app/services/feature-flags.js
(1 hunks)
💤 Files with no reviewable changes (1)
- app/components/course-page/course-stage-step/second-stage-tutorial-card.ts
🔇 Additional comments (4)
app/services/feature-flags.js (1)
17-19
: LGTM! Well-implemented feature flag.The implementation follows existing patterns and correctly handles null safety with the optional chaining operator.
app/components/course-page/course-stage-step/first-stage-tutorial-card/submit-code-step.ts (1)
4-7
: LGTM! Well-structured component changes.The changes follow TypeScript best practices, properly implement modal state management, and cleanly integrate the feature flag.
Also applies to: 12-13, 20-41
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs (1)
24-28
: LGTM! Clean template updates.The template changes correctly reflect the component interface updates with proper attribute passing.
app/components/course-page/course-stage-step/first-stage-tutorial-card/submit-code-step.hbs (1)
Line range hint
1-186
: Overall implementation looks good! 👍The split Git commands feature is well-implemented with comprehensive error handling and clear user guidance. The suggested improvements above are enhancements that can be addressed in follow-up PRs.
Checklist:
[percy]
in the message to trigger)Summary by CodeRabbit
New Features
Bug Fixes
Chores