Skip to content

Update README with getting started instructions#1

Draft
devendrapat wants to merge 2 commits intomainfrom
depa_propelai_testing
Draft

Update README with getting started instructions#1
devendrapat wants to merge 2 commits intomainfrom
depa_propelai_testing

Conversation

@devendrapat
Copy link
Owner

@devendrapat devendrapat commented Mar 13, 2026

Added instructions for getting started with the logging repository.
@auto

Added instructions for getting started with the logging repository.

Signed-off-by: devendrapat <[email protected]>
Signed-off-by: devendrapat <[email protected]>
@propel-code-bot
Copy link

Add brief README onboarding note and extend LoggingErrorCode enum

This PR adds a short introductory sentence in the ## 🚀 Getting Started section of README.md. It also introduces a new enum value kThisEnumIsOnlyForTesting in score/datarouter/error/error.h, expanding the LoggingErrorCode list.

This summary was automatically generated by @propel-code-bot

Copy link

@propel-code-bot propel-code-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found minor maintainability concerns about public API exposure and a syntax fix.

Status: Minor Suggestions | Risk: Medium

Issues Identified & Suggestions
  • Keep test-only enums out of public API contract: score/datarouter/error/error.h
  • Add missing comma to prevent compile error: score/datarouter/error/error.h
Review Details

📁 2 files reviewed | 💬 1 comments

👍 / 👎 individual comments to help improve reviews for you

kNoFileFound = 1,
kParseError,
kNoChannelsFound
kThisEnumIsOnlyForTesting

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommended

[Maintainability] The new enumerant kThisEnumIsOnlyForTesting is added to the public LoggingErrorCode contract even though it is strictly for tests. Because this header is the shipped error-domain interface, exposing a test-only value leaks test concerns across the API boundary and invites downstream code to depend on a code that has no runtime meaning, making later removal a breaking change. Keep test-only error codes in test-only headers or behind conditional compilation so the public API continues to describe only production errors.

Additionally, there's a missing comma after kNoChannelsFound on line 30 — without it, this code will not compile. If the test-only enumerant is retained (even temporarily), add the comma: kNoChannelsFound,.

Context for Agents
The new enumerant `kThisEnumIsOnlyForTesting` is added to the public `LoggingErrorCode` contract even though it is strictly for tests. Because this header is the shipped error-domain interface, exposing a test-only value leaks test concerns across the API boundary and invites downstream code to depend on a code that has no runtime meaning, making later removal a breaking change. Keep test-only error codes in test-only headers or behind conditional compilation so the public API continues to describe only production errors.

Additionally, there's a missing comma after `kNoChannelsFound` on line 30 — without it, this code will not compile. If the test-only enumerant is retained (even temporarily), add the comma: `kNoChannelsFound,`.

File: score/datarouter/error/error.h
Line: 31

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.

1 participant