Skip to content

fix compile #258

Merged
thweetkomputer merged 1 commit into
mainfrom
fix-compile-zc
Dec 3, 2025
Merged

fix compile #258
thweetkomputer merged 1 commit into
mainfrom
fix-compile-zc

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Dec 3, 2025

Copy link
Copy Markdown
Collaborator

Here are some reminders before you submit the pull request

  • Add tests for the change
  • Document changes
  • Reference the link of issue using fixes eloqdb/tx_service#issue_id
  • Reference the link of RFC if exists
  • Pass ./mtr --suite=mono_main,mono_multi,mono_basic

Summary by CodeRabbit

  • Chores
    • Updated internal build configuration to align logging dependencies with build-time feature flags.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 3, 2025

Copy link
Copy Markdown

Walkthrough

A preprocessor guard condition in the log initialization file is updated. AWS credential variable declarations shift from being conditionally compiled under DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3 to LOG_STATE_TYPE_RKDB_S3, aligning credential compilation with a different build-time feature flag.

Changes

Cohort / File(s) Change Summary
Preprocessor Guard Update
core/src/log_init.cpp
Replaced preprocessor guard from #if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) to #if defined(LOG_STATE_TYPE_RKDB_S3) for AWS credential variable declarations

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the new preprocessor flag LOG_STATE_TYPE_RKDB_S3 is correctly defined in build configuration
  • Confirm AWS credentials are still properly declared for all required build variants
  • Ensure no build regressions occur when toggling the feature flags

Suggested reviewers

  • liunyl
  • MrGuin

Poem

🐰 A guard in disguise, we wisely did swap,
From one flag to another, no logic to drop,
AWS keys now march to a new compile drum,
Build-time decisions—let the right features come! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description only contains an uncompleted checklist template with no actual context about the changes, missing required details about the issue being fixed. Complete the description by explaining what the compilation issue was, why the preprocessor guard change fixes it, and fill in the checklist items appropriately or reference the actual issue.
Title check ❓ Inconclusive The title 'fix compile' is vague and generic, using non-descriptive terms that don't clearly convey what compilation issue was fixed. Replace with a more specific title that describes the actual fix, such as 'Replace preprocessor guard for AWS credentials in log initialization' or 'Fix LOG_STATE_TYPE_RKDB_S3 condition in log_init.cpp'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-compile-zc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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)
core/src/log_init.cpp (1)

39-42: Guard macro now matches AWS S3 usage; fix looks correct

Switching the DECLARE_string(aws_access_key_id/secret_key) guard to #if defined(LOG_STATE_TYPE_RKDB_S3) aligns it with the later usage block that is also gated on LOG_STATE_TYPE_RKDB_S3, which should resolve the "undeclared FLAGS_aws_*" compilation issue for S3 log-state builds. As a minor optional tweak, you could mirror the nested condition from the usage site (e.g., also require LOG_STATE_TYPE_RKDB_CLOUD) to make the coupling even clearer, but it’s not necessary for correctness.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5934fc5 and affcdd9.

📒 Files selected for processing (1)
  • core/src/log_init.cpp (1 hunks)

@thweetkomputer thweetkomputer merged commit 18e273a into main Dec 3, 2025
4 checks passed
@thweetkomputer thweetkomputer deleted the fix-compile-zc branch December 3, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants