Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config template and telemetry fixes #377

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Config template and telemetry fixes #377

merged 1 commit into from
Oct 4, 2024

Conversation

MrTravisB
Copy link
Contributor

@MrTravisB MrTravisB commented Oct 4, 2024

Important

This PR adds configuration template parsing, new telemetry events for start/stop actions, and documentation formatting fixes.

  • Configuration:
    • Add parseConfigTemplate() in env_template.go to process environment variables in config templates.
    • Update Load() in load_ce.go to use parseConfigTemplate() for processing config files.
  • Telemetry:
    • Add Event_CEStart and Event_CEStop in events.go.
    • Update TrackEvent() in telemetry_ce.go to filter for CE events using isCEEvent().
    • Add isCEEvent() in telemetry_ce.go to check for CE-specific events.
    • Track Event_CEStart and Event_CEStop in setup_ce.go during setup and shutdown.
  • Documentation:
    • Fix formatting in README.md for code blocks and notes.

This description was created by Ellipsis for 0b12b06. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 0b12b06 in 12 seconds

More details
  • Looked at 163 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. src/lib/config/env_template.go:35
  • Draft comment:
    Typo in error message: 'environmentvariables' should be 'environment variables'.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The error message in parseConfigTemplate function has a typo in the word 'environmentvariables'. It should be 'environment variables'.
2. src/lib/config/env_template.go:10
  • Draft comment:
    The //nolint:unused comment is misleading as parseConfigTemplate is used in load_ce.go. Consider removing it.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The parseConfigTemplate function in env_template.go is marked as //nolint:unused, but it is actually used in load_ce.go. This comment is misleading and should be removed.
3. src/lib/telemetry/telemetry_ce.go:135
  • Draft comment:
    The isCEEvent function is only used once. Consider inlining it unless it will be reused.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The isCEEvent function is defined in telemetry_ce.go but is only used once in the same file. Consider inlining it to simplify the code unless it is expected to be used elsewhere in the future.

Workflow ID: wflow_NZaFBpYwIUc1ASRD


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@danielchalef danielchalef self-requested a review October 4, 2024 01:40
Copy link
Member

@danielchalef danielchalef left a comment

Choose a reason for hiding this comment

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

lgtm

@MrTravisB MrTravisB merged commit 9016d00 into main Oct 4, 2024
4 checks passed
@MrTravisB MrTravisB deleted the travis/1.0.1 branch October 4, 2024 01:43
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