Fix: change settings category from homepage to system#129
Conversation
|
❌ Found 0 compliant commit and 1 non-compliant commits in f1b03af. Commit f1b03af by @Amirhesp is not conform to the conventional commit specification :
|
WalkthroughUpdated the AndroidManifest fragment generated by build.rs for the system_settings feature: the meta-data value for com.android.settings.category was changed from com.android.settings.category.ia.homepage to com.android.settings.category.ia.system. No other changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
f1b03af to
7a58e55
Compare
|
@Amirhesp can you provide some more context about this PR? |
Hi @matthme, thanks for reviewing. |
|
@Amirhesp how does it relate to the already existing metadata field that gets added with the Wouldn't that cover what you need? It also says that this yaml file is auto-generated so are you sure it should be modified in that generated file? |
@matthme Both entries refer to the same metadata key com.android.settings.category, but with different category values: com.android.settings.category.ia.homepage: adds the entry to the Settings homepage/dashboard. com.android.settings.category.ia.system: categorizes it under the System section in Settings (used by AOSP Settings for System pages). |
|
Okay. I'm not very familiar with tauri 2 and android so apologize if I'm asking stupid questions. I may have wrong implicit assumptions.
I can see that but that doesn't fully answer my question. It seems like the values are contradicting each other then? Is your problem that the setting is currently showing up under "homepage" but it should show up under "system"? In that case it seems to me that it should get modified at the source in the Rust file, rather than downstream in the auto-generated yaml file (which I'm assuming is generated based on the Rust file). |
ٔNot stupid at all, you're right. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/android-service-runtime/src-tauri/build.rs (2)
19-20: Use a proper summary string, not the title, for Settings’ summary lineConsider pointing
com.android.settings.summaryto a dedicated summary string for better UX.If you already have a summary resource, update this line:
- <meta-data android:name="com.android.settings.summary" - android:resource="@string/main_activity_title" /> + <meta-data android:name="com.android.settings.summary" + android:resource="@string/settings_summary" />
1-3: Don’t forget the PR housekeeping (CHANGELOG + docs)Per the PR TODOs, update CHANGELOG(s) and regenerate docs. I can help draft the entry if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/android-service-runtime/src-tauri/build.rs(1 hunks)
🔇 Additional comments (2)
apps/android-service-runtime/src-tauri/build.rs (2)
17-18: Category switched to ia.system — aligns with desired placement under Settings > SystemUsing
com.android.settings.category.ia.systemwith theIA_SETTINGSaction is the right combo for surfacing the activity under the System section. No runtime impact; manifest-only.
4-23: No duplicate metadata detectedI’ve scanned every AndroidManifest.xml and the repository for
com.android.settings.categoryandcom.android.settings.action.IA_SETTINGS, and the only occurrences are in thebuild.rsfragment under thesystem_settingsfeature. There are no legacyia.homepageinjections or other manifest entries that could collide, and thesystem_settings = []feature is correctly declared inapps/android-service-runtime/src-tauri/Cargo.toml. The final merged manifest will therefore contain exactly onecom.android.settings.category(with valueia.system).
|
@Amirhesp great, thank you 👍 . One more question, does the actual AndroidManifest.yml need to get rebuilt and commited now? |
Yes, it does. |
Summary
TODO:
pnpm run build:doc)Summary by CodeRabbit