-
Notifications
You must be signed in to change notification settings - Fork 934
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
refactor: change metric config to url like other module #2396
Conversation
c8d1b76
to
fb75240
Compare
Codecov Report
@@ Coverage Diff @@
## main #2396 +/- ##
==========================================
+ Coverage 44.43% 44.88% +0.44%
==========================================
Files 303 307 +4
Lines 18402 18795 +393
==========================================
+ Hits 8177 8436 +259
- Misses 9360 9479 +119
- Partials 865 880 +15
... and 8 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
What is the benifit of using I am thinking the If you still want to use a type of |
Thank you for your reply, the reason is
|
c174495
to
69812f2
Compare
Kudos, SonarCloud Quality Gate passed! |
Both ways can meet the requirements of passing metrics configurations for now. I think URL as the current unified design for now is acceptable. We can revisit the design later in the following refactor plan. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
metrics.ReporterConfig
will be removed in the futureThe configuration class of the metric module is a copy of the configuration class of the config package. This commit will change the configuration of the metric module to be dependent on
common.URL
, in order to have a consistent style with other modules.