feat(iOS): settings #51
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
krystxf
added
enhancement
krystxf
force-pushed
the
feat/app-settings
branch
from
bb25cc5 to
86713eb
Compare
WalkthroughThis pull request introduces a series of changes to the The user interface has been updated with the introduction of a new Possibly related PRs
Warning Rate limit exceeded@krystxf has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 (
|
![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 (10)
apps/mobile/metro-now/metro-now/utils/app-storage-keys.enum.swift (2)
6-6: LGTM! Consider adding documentation.The new case follows Swift naming conventions and clearly represents its purpose. Consider adding a documentation comment to describe its usage.
enum AppStorageKeys: String { case hasSeenWelcomeScreen + /// Controls whether to show only metro services in the app case showMetroOnly }
4-7: Consider organizing settings keys as the app grows.As more settings are added, consider grouping related keys into sections using nested enums or a structured approach to maintain scalability.
Example structure for future consideration:
enum AppStorageKeys: String { enum General: String { case showMetroOnly } enum Onboarding: String { case hasSeenWelcomeScreen } }apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-general-section.view.swift (3)
1-4: Consider enhancing file documentation.While the GitHub repository link is helpful, consider adding:
- Copyright notice
- Brief description of the view's purpose and functionality
- Any important usage notes or dependencies
// metro-now // https://github.com/krystxf/metro-now +// +// Copyright © 2024 krystxf. All rights reserved. +// +// Settings page general section view that provides metro-only filtering option +// within the app's settings page. import SwiftUI
11-23: Consider localizing text strings.The view contains hardcoded strings that should be localized for better internationalization support.
var body: some View { Section( header: - Label("General", systemImage: "gear"), + Label(NSLocalizedString("settings.general.title", comment: "General settings section title"), + systemImage: "gear"), footer: - Text("Doesn't show buses or any other means of transport, just metro.") + Text(NSLocalizedString("settings.general.metro_only.description", + comment: "Description of metro-only filter")) ) { Toggle(isOn: $showMetroOnly) { - Text("Metro only") + Text(NSLocalizedString("settings.general.metro_only.toggle", + comment: "Toggle for metro-only filter")) } } }
25-29: Enhance preview with different contexts.Consider expanding the preview to demonstrate the view in different states and color schemes.
#Preview { - List { - SettingsPageGeneralSectionView() + Group { + List { + SettingsPageGeneralSectionView() + } + .previewDisplayName("Light Mode") + + List { + SettingsPageGeneralSectionView() + } + .preferredColorScheme(.dark) + .previewDisplayName("Dark Mode") } }apps/mobile/metro-now/metro-now/pages/welcome/welcome-page.view.swift (1)
62-62: Enhance preview with meaningful dismiss handlerThe preview could be more useful with a demonstration of the dismiss functionality.
Consider updating the preview like this:
- WelcomePageView {} + WelcomePageView { + print("Welcome page dismissed") + }apps/mobile/metro-now/metro-now/ContentView.swift (1)
19-27: LGTM! Consider adding accessibility identifier.The toolbar implementation follows SwiftUI best practices and uses standard system icons. The placement is consistent with iOS design guidelines.
Consider adding an accessibility identifier to help with UI testing:
NavigationLink { SettingsPageView() } label: { Label("Settings", systemImage: "gearshape") + .accessibilityIdentifier("settingsButton") }apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift (3)
6-16: Consider moving URLs to a configuration fileThe hardcoded URLs would be better maintained in a dedicated configuration file, making it easier to update them across the app.
Consider creating a dedicated configuration enum or struct:
enum AppURLs { static let reportBug = URL(string: "https://github.com/krystxf/metro-now/issues/new")! .appending(queryItems: [ URLQueryItem(name: "template", value: "bug_report.md"), URLQueryItem(name: "title", value: "") ]) // ... other URLs }
34-38: Add user feedback for rating actionConsider providing visual feedback when the rating action is triggered.
Label("Rate the app", systemImage: "star") .foregroundStyle(.link) .onTapGesture { requestReview() + // Add haptic feedback + UIImpactFeedbackGenerator(style: .light).impactOccurred() }
61-65: Enhance preview with multiple statesConsider adding more preview scenarios to test different states and configurations.
#Preview { - List { - SettingsPageAboutSectionView() - } + Group { + List { + SettingsPageAboutSectionView() + } + .previewDisplayName("Default") + + List { + SettingsPageAboutSectionView() + } + .preferredColorScheme(.dark) + .previewDisplayName("Dark Mode") + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj(0 hunks)apps/mobile/metro-now/metro-now/ContentView.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-footer.view.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-general-section.view.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/settings-page.view.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/utils/bundle.utils.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/welcome/welcome-page.view.swift(2 hunks)apps/mobile/metro-now/metro-now/utils/app-storage-keys.enum.swift(1 hunks)
💤 Files with no reviewable changes (1)
- apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj
✅ Files skipped from review due to trivial changes (3)
- apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-footer.view.swift
- apps/mobile/metro-now/metro-now/pages/settings/settings-page.view.swift
- apps/mobile/metro-now/metro-now/pages/settings/utils/bundle.utils.swift
🔇 Additional comments (5)
apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-general-section.view.swift (1)
6-10: Verify AppStorageKeys enum implementation.
The code references AppStorageKeys.showMetroOnly, but we need to ensure it's properly defined.
✅ Verification successful
AppStorageKeys enum is properly defined and includes the required case
The AppStorageKeys enum is correctly defined in app-storage-keys.enum.swift with the showMetroOnly case, confirming proper implementation for the @AppStorage wrapper usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for AppStorageKeys enum definition
ast-grep --pattern 'enum AppStorageKeys {
$$$
case showMetroOnly
$$$
}'
# Backup search using ripgrep
rg -p "enum AppStorageKeys" -A 10
Length of output: 274
apps/mobile/metro-now/metro-now/pages/welcome/welcome-page.view.swift (1)
12-12: Verify all references to the renamed view
The rename from WelcomeScreenView to WelcomePageView improves consistency with the file naming convention.
Let's verify all references have been updated:
✅ Verification successful
All references to the renamed view have been properly updated
The verification confirms that:
- No instances of the old name
WelcomeScreenViewremain in the codebase - The new name
WelcomePageViewis correctly used in:ContentView.swift(as a component)welcome-page.view.swift(component definition and preview)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old name
rg "WelcomeScreenView" --type swift
# Search for the new name to confirm usage
rg "WelcomePageView" --type swift
Length of output: 382
apps/mobile/metro-now/metro-now/ContentView.swift (1)
33-33: Verify consistent renaming across the codebase.
The rename from WelcomeScreenView to WelcomePageView looks good, but let's ensure all references have been updated.
✅ Verification successful
All references to WelcomeScreenView have been properly updated to WelcomePageView
The search results confirm that there are no remaining references to WelcomeScreenView in the codebase, and WelcomePageView is consistently used in both the component definition and its usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to WelcomeScreenView
# that might have been missed during the rename
# Search for any remaining occurrences of WelcomeScreenView
rg "WelcomeScreenView" --type swift
# Search for the new name to confirm consistent usage
rg "WelcomePageView" --type swift
Length of output: 382
apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift (2)
1-4: LGTM: Clean header and appropriate import
The file header and import statement are correctly structured.
18-59: Verify integration with main settings page
Let's ensure this component is properly integrated into the main settings page.
✅ Verification successful
Let me check the integration in the main settings page to ensure proper usage.
Integration verified successfully
The SettingsPageAboutSectionView is properly integrated into the main settings page as a section within the List view, alongside other sections like SettingsPageGeneralSectionView. The implementation follows SwiftUI's compositional pattern and is correctly positioned in the view hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of SettingsPageAboutSectionView in other files
rg "SettingsPageAboutSectionView" --type swift
Length of output: 442
Script:
#!/bin/bash
# Get the content of settings page to verify integration
cat apps/mobile/metro-now/metro-now/pages/settings/settings-page.view.swift
Length of output: 609
.../mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift (2)
Line range hint
24-42: Consider refactoring the filtering logic for better maintainability.The nested filtering logic could be extracted into a separate function to improve readability and maintainability.
Consider this refactoring:
+ private func filterNonMetroPlatforms(_ platforms: [Platform]) -> [Platform] { + platforms.filter { platform in + platform.routes.contains { route in + let routeName = route.name.uppercased() + return !METRO_LINES.contains(routeName) + } + }.sorted(by: { getPlatformLabel($0) < getPlatformLabel($1) }) + } if !showMetroOnly, let closestStop = viewModel.closestStop { - let platforms = closestStop.platforms - .filter { platform in - platform.routes.contains(where: { - let routeName = $0.name.uppercased() - let containsMetro = METRO_LINES.contains(routeName) - return !containsMetro - }) - } - .sorted(by: { - getPlatformLabel($0) < getPlatformLabel($1) - }) + let platforms = filterNonMetroPlatforms(closestStop.platforms)
Line range hint
1-56: Consider splitting the view into smaller, more focused components.The view currently handles both metro and non-metro stops, which could be split into separate view components for better maintainability and reusability. This would also make the code easier to test and modify in the future.
Consider creating:
MetroStopsViewNonMetroStopsViewThis would reduce the complexity of the main view and make the codebase more modular.
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page-view-model.swift (1)
41-48: Consider consolidating duplicate logic with startPeriodicRefresh()The new
refresh()method duplicates the stop ID collection logic fromstartPeriodicRefresh(). Consider extracting this common functionality into a private method for better maintainability.+ private func getStopIds() -> [String] { + guard let closestMetroStop, let closestStop else { + return [] + } + return [closestMetroStop.id, closestStop.id] + } + func refresh() { - let stopIds = [closestMetroStop?.id ?? "", closestStop?.id ?? ""] - - getDepartures( - stopsIds: stopIds.filter { !$0.isEmpty }, - platformsIds: [] - ) + let stopIds = getStopIds() + if !stopIds.isEmpty { + getDepartures(stopsIds: stopIds, platformsIds: []) + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page-view-model.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift(3 hunks)
🔇 Additional comments (2)
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift (1)
8-10: LGTM! Well-implemented persistent storage.
The @AppStorage property is properly implemented with a clear key reference and sensible default value.
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page-view-model.swift (1)
41-48: 🛠️ Refactor suggestion
Add error handling for edge cases
The current implementation might make an unnecessary API call with an empty array if both stops are nil. Consider adding appropriate error handling or state management.
Let's verify the potential for this edge case by checking the view's usage:
func refresh() {
- let stopIds = [closestMetroStop?.id ?? "", closestStop?.id ?? ""]
-
- getDepartures(
- stopsIds: stopIds.filter { !$0.isEmpty },
- platformsIds: []
- )
+ guard let metroStopId = closestMetroStop?.id else {
+ // Consider publishing an error state if needed
+ return
+ }
+
+ var stopIds = [metroStopId]
+ if let regularStopId = closestStop?.id {
+ stopIds.append(regularStopId)
+ }
+
+ getDepartures(stopsIds: stopIds, platformsIds: [])
}
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift
Outdated
Show resolved
Hide resolved
krystxf
force-pushed
the
feat/app-settings
branch
from
40a5c61 to
1b77771
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift (1)
24-48: Consider refactoring for improved maintainability.The current implementation has several areas that could be improved:
- Complex nested conditionals could be simplified
- Platform filtering logic could be extracted to a separate function
- Button text should be localized for internationalization
Consider this refactoring:
-if showMetroOnly { - Button("Show all public transport departures") { - showMetroOnly.toggle() - } -} else { - if let closestStop = viewModel.closestStop { - let platforms = closestStop.platforms - .filter { platform in - platform.routes.contains(where: { - let routeName = $0.name.uppercased() - let containsMetro = METRO_LINES.contains(routeName) - return !containsMetro - }) - } - .sorted(by: { - getPlatformLabel($0) < getPlatformLabel($1) - }) +if let closestStop = viewModel.closestStop { + Button(LocalizedStringKey("show.all.transport.departures")) { + showMetroOnly.toggle() + } + .opacity(showMetroOnly ? 1 : 0) + + if !showMetroOnly { + ForEach(filterNonMetroPlatforms(closestStop.platforms), id: \.id) { platform in + PlatformDeparturesListView( + platform: platform, + departures: viewModel.departures + ) + } + } +}Add this helper function:
private func filterNonMetroPlatforms(_ platforms: [Platform]) -> [Platform] { platforms .filter { platform in platform.routes.contains { route in !METRO_LINES.contains(route.name.uppercased()) } } .sorted(by: { getPlatformLabel($0) < getPlatformLabel($1) }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift(2 hunks)apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift
🔇 Additional comments (2)
apps/mobile/metro-now/metro-now/pages/closest-stop/closest-stop-page.view.swift (2)
8-10: LGTM! Well-structured persistent storage implementation.
The @AppStorage property is properly implemented using an enumerated key, following SwiftUI best practices for user preferences.
53-59: Previous review comment about error handling is still applicable.
The refreshable implementation still contains the debug print and lacks proper error handling.
krystxf
force-pushed
the
feat/app-settings
branch
from
0d1c73c to
4c911d1
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
apps/mobile/metro-now/metro-now/pages/settings/utils/get-formatted-version-number.swift (1)
6-23: Add documentation and consider a more concise implementationThe function logic is correct, but could benefit from documentation and a more concise implementation.
Consider this improved version:
+/// Returns a formatted version string combining the app's version and build numbers. +/// - Returns: A string in one of these formats: +/// - "1.0.0(123)" when both version and build numbers are available +/// - "1.0.0" when only version number is available +/// - "(123)" when only build number is available +/// - "unknown version" when neither is available func getFormattedVersionNumber() -> String { - let version = Bundle.main.versionNumber - let build = Bundle.main.buildNumber - - guard let version else { - if let build { - return "(\(build))" - } else { - return "unknown version" - } - } - - if let build { - return "\(version)(\(build))" - } - - return version + let version = Bundle.main.versionNumber + let build = Bundle.main.buildNumber + + switch (version, build) { + case (let v?, let b?): return "\(v)(\(b))" + case (let v?, nil): return v + case (nil, let b?): return "(\(b))" + case (nil, nil): return "unknown version" + } }apps/mobile/metro-now/metro-now/pages/settings/utils/github.utils.swift (3)
1-4: Enhance file documentationConsider adding a brief description of the file's purpose and its role in the settings functionality.
// metro-now // https://github.com/krystxf/metro-now +// Utilities for generating GitHub issue URLs used in the settings page +// for bug reports and feature requests.
6-9: Add documentation and specify access levelThe enum should be documented and have an explicit access level for better API clarity.
+/// Represents available GitHub issue template types +public enum GithubIssueTemplate: String { -enum GithubIssueTemplate: String { case bug_report case feature_request }
12-12: Refactor URL construction for better maintainabilityThe hardcoded repository URL and empty query parameters could be improved:
- Consider making the repository URL configurable
- Remove empty query parameters unless they serve a specific purpose
+private let githubRepoUrl = "https://github.com/krystxf/metro-now" + func getGithubIssueUrl(template: GithubIssueTemplate, title: String = "") -> URL? { - let url = URL(string: "https://github.com/krystxf/metro-now/issues/new") + let url = URL(string: "\(githubRepoUrl)/issues/new") guard let url else { return nil } let urlWithQueryParams = url.appending(queryItems: [ - URLQueryItem(name: "assignees", value: nil), - URLQueryItem(name: "labels", value: nil), - URLQueryItem(name: "projects", value: nil), URLQueryItem(name: "template", value: "\(template.rawValue).md"), URLQueryItem(name: "title", value: title), ])Also applies to: 18-24
🛑 Comments failed to post (3)
apps/mobile/metro-now/metro-now/pages/settings/utils/get-formatted-version-number.swift (1)
14-14:
⚠️ Potential issueFix typo in error message
The word "unknown" is misspelled as "uknknown".
- return "uknknown version" + return "unknown version"📝 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.return "unknown version"apps/mobile/metro-now/metro-now/pages/settings/utils/github.utils.swift (1)
11-27: 🛠️ Refactor suggestion
Add documentation and improve function signature
The function should be documented and have an explicit access level.
+/// Generates a URL for creating a new GitHub issue +/// - Parameters: +/// - template: The type of issue template to use +/// - title: Optional pre-filled issue title +/// - Returns: A URL to create a new GitHub issue, or nil if URL creation fails +public func getGithubIssueUrl(template: GithubIssueTemplate, title: String = "") -> URL? { -func getGithubIssueUrl(template: GithubIssueTemplate, title: String = "") -> URL? {📝 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./// Generates a URL for creating a new GitHub issue /// - Parameters: /// - template: The type of issue template to use /// - title: Optional pre-filled issue title /// - Returns: A URL to create a new GitHub issue, or nil if URL creation fails public func getGithubIssueUrl(template: GithubIssueTemplate, title: String = "") -> URL? { let url = URL(string: "https://github.com/krystxf/metro-now/issues/new") guard let url else { return nil } let urlWithQueryParams = url.appending(queryItems: [ URLQueryItem(name: "assignees", value: nil), URLQueryItem(name: "labels", value: nil), URLQueryItem(name: "projects", value: nil), URLQueryItem(name: "template", value: "\(template.rawValue).md"), URLQueryItem(name: "title", value: title), ]) return urlWithQueryParams }apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj (1)
468-468: 💡 Codebase verification
Version updates are inconsistent across targets
The version numbers are not consistently updated across all targets in the project:
- Main app and Watch app targets are updated to version 0.3.3 with build number 3
- Test targets remain at version 1.0 with build number 1
This inconsistency should be addressed by updating all targets to maintain version parity.
🔗 Analysis chain
Version updates look good
Changes include:
- Incremented CURRENT_PROJECT_VERSION from 2 to 3
- Updated MARKETING_VERSION from 0.3.2 to 0.3.3
- Standardized WATCHOS_DEPLOYMENT_TARGET to 10.0
The version bump follows semantic versioning for a feature addition (settings page).
Also applies to: 482-482, 490-490, 500-500, 514-514, 523-523, 533-533, 551-551, 567-567, 585-585
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify version consistency across Info.plist files # Check for version numbers in Info.plist files fd -e plist -x grep -l "CFBundleShortVersionString\|CFBundleVersion" {}Length of output: 75
Script:
#!/bin/bash # Let's try a different approach to find Info.plist files and check their contents # First find all plist files fd -e plist | while read -r file; do echo "=== $file ===" cat "$file" | grep -A1 -E "CFBundleShortVersionString|CFBundleVersion|MARKETING_VERSION|CURRENT_PROJECT_VERSION" doneLength of output: 28
Script:
#!/bin/bash # Let's examine the project.pbxproj file directly to verify version numbers cat "apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj" | grep -A1 -E "CURRENT_PROJECT_VERSION|MARKETING_VERSION|WATCHOS_DEPLOYMENT_TARGET"Length of output: 1372
krystxf
force-pushed
the
feat/app-settings
branch
from
4c911d1 to
89a8557
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj (1)
492-492: Consider specifying minor version for watchOS deployment targetThe watchOS deployment target is set to 10.0. While this is valid, consider being more specific about the minor version (e.g., 10.1) to ensure compatibility with specific watchOS features and bug fixes.
Also applies to: 525-525
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/mobile/metro-now/common/const/github.const.swift(1 hunks)apps/mobile/metro-now/common/const/review-url.const.swift(1 hunks)apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj(11 hunks)apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-footer.view.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/utils/get-formatted-version-number.swift(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/utils/github.utils.swift(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/mobile/metro-now/common/const/github.const.swift
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/mobile/metro-now/common/const/review-url.const.swift
- apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-about-section.view.swift
- apps/mobile/metro-now/metro-now/pages/settings/components/settings-page-footer.view.swift
- apps/mobile/metro-now/metro-now/pages/settings/utils/get-formatted-version-number.swift
- apps/mobile/metro-now/metro-now/pages/settings/utils/github.utils.swift
🔇 Additional comments (2)
apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj (2)
470-470: Version bump follows semantic versioning
The version changes look appropriate:
- CURRENT_PROJECT_VERSION: 2 → 3
- MARKETING_VERSION: 0.3.2 → 0.3.3
The patch version increment (0.3.2 → 0.3.3) aligns with adding a new feature (settings page) that doesn't break backward compatibility.
Also applies to: 484-484, 492-492, 502-502, 516-516, 525-525, 535-535, 553-553, 569-569, 587-587
60-61: Verify the purpose of new const files
The addition of github.const.swift and review-url.const.swift to membership exceptions suggests integration with GitHub/review functionality. However, these files seem unrelated to the PR's objective of adding a settings page.
Also applies to: 78-79
✅ Verification successful
Based on the provided output, I can now verify the purpose of the const files. The files github.const.swift and review-url.const.swift contain constants that are used in the settings page functionality:
github.const.swiftdefinesGITHUB_URLwhich is used inSettingsPageFooterViewandgetGithubIssueUrlfunction for source code link and issue reporting.review-url.const.swiftdefinesREVIEW_URLwhich is used inSettingsPageAboutSectionViewfor the "Give us a rating" link.
The const files are correctly related to the PR's objective
These const files are integral to the settings page feature as they provide necessary URLs for the settings page functionality, specifically in the About section where users can:
- View source code
- Report bugs
- Request features
- Rate the app
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the content and usage of these const files
fd -e swift -E "github.const.swift|review-url.const.swift" --exec cat {}
Length of output: 63769
Changes