chore(sky-sync): move to core#2418
Conversation
📝 WalkthroughWalkthroughSkySync is promoted to a core feature by adding an ChangesSkySync Core Feature Promotion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): [00.12][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/SkySync.h 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 |
|
No actionable suggestions for changed features. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Features/SkySync.h (1)
50-50: Consider adding a linked issue in the PR descriptionSince this change implements a feature-level behavior change, add a reference like
Implements #<id>orAddresses #<id>if a tracking issue exists.As per coding guidelines, "When reviewing PRs, please provide suggestions for ... Issue References ... 'Implements
#123' or 'Addresses#123' for features".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Features/SkySync.h` at line 50, This change to the feature flag method IsCore() in src/Features/SkySync.h introduces feature-level behavior — update the pull request description to include a tracking issue reference by adding a line such as "Implements #<id>" or "Addresses #<id>" (replace <id> with the issue number) so the SkySync::IsCore() behavioral change is linked to the appropriate issue per guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Features/SkySync.h`:
- Line 50: This change to the feature flag method IsCore() in
src/Features/SkySync.h introduces feature-level behavior — update the pull
request description to include a tracking issue reference by adding a line such
as "Implements #<id>" or "Addresses #<id>" (replace <id> with the issue number)
so the SkySync::IsCore() behavioral change is linked to the appropriate issue
per guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9599fd2b-49a5-4a86-be94-0dd708ce2e05
📒 Files selected for processing (3)
features/Sky Sync/COREfeatures/Sky Sync/Shaders/Features/SkySync.inisrc/Features/SkySync.h
💤 Files with no reviewable changes (1)
- features/Sky Sync/Shaders/Features/SkySync.ini
|
✅ A pre-release build is available for this PR: |
SkrubbySkrubInAShrub
force-pushed
the
Sky-Sync-Core
branch
from
d341d8b to
0f180d2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Features/SkySync.h (1)
7-7: ⚡ Quick winConsider removing now-unused Nexus-related code.
With
IsCore()returningtrue, the system will no longer call this feature'sGetFeatureModLink()method (perFeatureIssues::GetFeatureModLink()in src/FeatureIssues.cpp:734-741). TheMOD_IDconstant andGetFeatureModLink()override are now dead code and can be removed for consistency with the Nexus metadata removal from the INI file.♻️ Proposed cleanup
- static constexpr std::string_view MOD_ID = "153543"; - public: virtual inline std::string GetName() override { return "Sky Sync"; } virtual inline std::string GetShortName() override { return "SkySync"; } - virtual inline std::string GetFeatureModLink() override { return MakeNexusModURL(MOD_ID); } virtual std::string_view GetCategory() const override { return FeatureCategories::kSky; }Also applies to: 12-12
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Features/SkySync.h` at line 7, Remove the now-dead Nexus metadata by deleting the static constexpr std::string_view MOD_ID and the GetFeatureModLink() override from the SkySync feature class (they are unused because IsCore() returns true and FeatureIssues::GetFeatureModLink() will no longer call this feature). Search for and remove any references to MOD_ID/GetFeatureModLink() in the feature implementation to avoid dangling symbols, rebuild to ensure no remaining references, and update includes or forward declarations if the removals expose unused headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Features/SkySync.h`:
- Line 7: Remove the now-dead Nexus metadata by deleting the static constexpr
std::string_view MOD_ID and the GetFeatureModLink() override from the SkySync
feature class (they are unused because IsCore() returns true and
FeatureIssues::GetFeatureModLink() will no longer call this feature). Search for
and remove any references to MOD_ID/GetFeatureModLink() in the feature
implementation to avoid dangling symbols, rebuild to ensure no remaining
references, and update includes or forward declarations if the removals expose
unused headers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e59697a7-0dac-4ed2-a5a5-c40aa225696b
📒 Files selected for processing (3)
features/Sky Sync/COREfeatures/Sky Sync/Shaders/Features/SkySync.inisrc/Features/SkySync.h
💤 Files with no reviewable changes (1)
- features/Sky Sync/Shaders/Features/SkySync.ini
2 participants
Summary by CodeRabbit