Skip to content

Add comment for the plugins field in generic application config #5801

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

Merged

Conversation

ffjlabo
Copy link
Member

@ffjlabo ffjlabo commented May 7, 2025

What this PR does:

as titlw

Why we need it:

We want to clarify why defined spec.plugins are empty struct.

Which issue(s) this PR fixes:

Follow #5789

Does this PR introduce a user-facing change?: no

  • How are users affected by this change: no
  • Is this breaking change: no
  • How to migrate (if breaking change):

Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.14%. Comparing base (e54cd83) to head (2dba71d).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5801      +/-   ##
==========================================
- Coverage   27.14%   27.14%   -0.01%     
==========================================
  Files         506      508       +2     
  Lines       53705    53803      +98     
==========================================
+ Hits        14577    14603      +26     
- Misses      38013    38084      +71     
- Partials     1115     1116       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@@ -59,6 +59,7 @@ type GenericApplicationSpec struct {
// Configuration for drift detection
DriftDetection *DriftDetection `json:"driftDetection"`
// List of the plugin name
// This field is plugin-specific, so intentionally restrict the access for the actual value here and decode it on the SDK side.
Copy link
Member

Choose a reason for hiding this comment

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

So we should rewrite the above line either. Since it's not just list of plugin name anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed 2dba71d

Signed-off-by: Yoshiki Fujikane <[email protected]>
@ffjlabo ffjlabo requested a review from khanhtc1202 May 8, 2025 01:13
@ffjlabo ffjlabo enabled auto-merge (squash) May 8, 2025 03:35
Copy link
Member

@Warashi Warashi left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@ffjlabo ffjlabo merged commit ba27d94 into master May 12, 2025
19 checks passed
@ffjlabo ffjlabo deleted the add-comment-for-plugins-field-in-generic-application-config branch May 12, 2025 00:57
@github-actions github-actions bot mentioned this pull request May 15, 2025
@github-actions github-actions bot mentioned this pull request May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants