-
Notifications
You must be signed in to change notification settings - Fork 5
RR-T39 Fixing the ci build #83
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
Conversation
WalkthroughUpdates a GitHub Actions workflow step to continue on HTTP failures and adds a ScrollView wrapper around the login form in the React Native app, changing rendering/scroll behavior without altering form logic. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CI as GitHub Actions
participant Step as "Send Release Notes"
rect `#DFF2E1`
CI->>Step: POST release notes
end
alt HTTP 2xx
Step-->>CI: 2xx (success)
CI->>CI: continue workflow
else HTTP non-2xx
rect `#FFF3CD`
Step-->>CI: non-2xx (error)
CI->>CI: log message ("non-2xx response, continuing")
CI->>CI: continue workflow (no exit)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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: 0
🧹 Nitpick comments (1)
.github/workflows/react-native-cicd.yml (1)
377-426: Redundant error-handling mechanisms; consider simplifying.The step uses both
continue-on-error: true(line 379) and removes theexit 1logic (line 425), which is redundant. Withset -eand noexit 1in the else branch, the script exits successfully (status 0) on Changerawr failures. Thecontinue-on-error: trueonly has an effect if the step fails (non-zero exit), which won't happen here.Choose one approach:
- Option A (simpler): Remove
continue-on-error: true; rely on the missingexit 1to allow the workflow to proceed.- Option B (more explicit): Keep
continue-on-error: trueand restoreexit 1to make failures visible; GitHub Actions will log the failure but continue the workflow.Option B is recommended for better observability—explicit failure signals in logs are easier to monitor and alert on than silent success.
- name: 📡 Send Release Notes to Changerawr if: ${{ matrix.platform == 'android' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'prod-apk') }} - continue-on-error: true run: | set -eo pipefail ... else echo "⚠️ Failed to send release notes to Changerawr (HTTP $HTTP_STATUS)" cat /tmp/changerawr_response.json + exit 1 fi + continue-on-error: trueThis makes the intent clearer: the step explicitly fails on Changerawr errors, but GitHub Actions allows the workflow to proceed thanks to
continue-on-error: true.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/react-native-cicd.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
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 (1)
src/app/login/login-form.tsx (1)
57-65: Consider memoizing all event handlers consistently.
handleServerUrlPressusesuseCallbackfor optimization, buthandleStateandhandleKeyPressdon't. For consistency and to prevent potential re-renders, consider wrapping all event handlers inuseCallback.Apply this diff to memoize the remaining handlers:
- const handleState = () => { + const handleState = useCallback(() => { setShowPassword((showState) => { return !showState; }); - }; + }, []); + - const handleKeyPress = () => { + const handleKeyPress = useCallback(() => { Keyboard.dismiss(); handleSubmit(onSubmit)(); - }; + }, [handleSubmit, onSubmit]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/login/login-form.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/app/login/login-form.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/login/login-form.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/app/login/login-form.tsx
🧠 Learnings (6)
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
Repo: Resgrid/Responder PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Ensure UI is intuitive, user-friendly, and works across different devices and screen sizes
Applied to files:
src/app/login/login-form.tsx
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
Repo: Resgrid/Responder PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Ensure responsive design across screen sizes and orientations
Applied to files:
src/app/login/login-form.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
Repo: Resgrid/Responder PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Ensure responsive design for different screen sizes and orientations
Applied to files:
src/app/login/login-form.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
Repo: Resgrid/Responder PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Applied to files:
src/app/login/login-form.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
Repo: Resgrid/Responder PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use react-hook-form for form handling
Applied to files:
src/app/login/login-form.tsx
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
Repo: Resgrid/Responder PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.{ts,tsx} : Use react-hook-form for form handling
Applied to files:
src/app/login/login-form.tsx
🧬 Code graph analysis (1)
src/app/login/login-form.tsx (2)
src/components/ui/index.tsx (2)
ScrollView(19-19)View(19-19)src/components/ui/image.tsx (1)
Image(12-14)
🪛 GitHub Check: test
src/app/login/login-form.tsx
[warning] 42-42:
Delete ·
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
src/app/login/login-form.tsx (3)
8-8: Import addition looks good.The ScrollView import is necessary for the layout enhancement and is correctly imported from react-native.
80-166: ScrollView wrapper improves form accessibility on smaller screens.The ScrollView addition with appropriate keyboard handling props (
keyboardShouldPersistTaps="handled",contentContainerStyle={{ flexGrow: 1 }}) ensures the login form remains accessible and usable across different screen sizes. The existing form logic, validation, and submission flow are preserved correctly.
42-42: Verify: theerrorprop is never displayed to users.The component receives an
errorprop (typed asstring | undefined) but doesn't render it anywhere. If this is meant to display server-side authentication errors to users, you'll need to add error display logic.If the error should be displayed, consider adding an error alert above or below the form:
{error ? ( <View className="mb-4 rounded-md bg-red-50 p-3"> <Text className="text-red-800">{error}</Text> </View> ) : null}Is the omission intentional, or should authentication errors be displayed to users?
| }; | ||
|
|
||
| export const LoginForm = ({ onSubmit = () => {}, isLoading = false, error = undefined }: LoginFormProps) => { | ||
| export const LoginForm = ({ onSubmit = () => { }, isLoading = false, error = undefined }: LoginFormProps) => { |
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.
Remove extra space in empty arrow function.
Static analysis correctly flags a spacing issue in the default value for onSubmit.
Apply this diff to fix the formatting:
-export const LoginForm = ({ onSubmit = () => { }, isLoading = false, error = undefined }: LoginFormProps) => {
+export const LoginForm = ({ onSubmit = () => {}, isLoading = false, error = undefined }: LoginFormProps) => {📝 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.
| export const LoginForm = ({ onSubmit = () => { }, isLoading = false, error = undefined }: LoginFormProps) => { | |
| export const LoginForm = ({ onSubmit = () => {}, isLoading = false, error = undefined }: LoginFormProps) => { |
🧰 Tools
🪛 GitHub Check: test
[warning] 42-42:
Delete ·
🤖 Prompt for AI Agents
In src/app/login/login-form.tsx around line 42, the default value for the
onSubmit prop uses an empty arrow function with extra space inside its block
("() => { }"); remove the extra space so the empty function is formatted as "()
=> {}" to satisfy the linter/formatter.
|
Approve |
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.
This PR is approved.
Summary by CodeRabbit