Conversation
Signed-off-by: Patrick Knight <pknight@redhat.com>
| { key: 'error', expression: '==false' }, | ||
| ], | ||
| }; | ||
|
|
There was a problem hiding this comment.
This could also be a config option similar to the other providers like the following:
thresholds?: {
rules?: Array<{
key: 'error' | 'success';
/** Threshold expression - supports: ==true, ==false*/
expression: string;
}>;
};
in the event that we would like to grant more options to the user
| /** | ||
| * Get all metric IDs this provider handles. | ||
| * For batch providers that handle multiple metrics. | ||
| * Defaults to [getProviderId()] if not implemented. | ||
| * @public | ||
| */ | ||
| getMetricIds?(): string[]; | ||
|
|
||
| /** | ||
| * Get all metrics this provider exposes. | ||
| * For batch providers that handle multiple metrics. | ||
| * Defaults to [getMetric()] if not implemented. | ||
| * @public | ||
| */ | ||
| getMetrics?(): Metric<T>[]; | ||
|
|
||
| /** | ||
| * Calculate multiple metrics in a single call. | ||
| * For batch providers that can efficiently compute multiple metrics together. | ||
| * Returns a Map of metric ID to calculated value. | ||
| * If not implemented, falls back to calculateMetric(). | ||
| * @public | ||
| */ | ||
| calculateMetrics?(entity: Entity): Promise<Map<string, MetricValue<T>>>; |
There was a problem hiding this comment.
I know we mentioned we wouldn't do this, but adding optional getMetrics() and calculateMetrics() methods enables some future efficiency gains: should help in avoiding rate limits. Since they're optional, existing providers work unchanged with minimal core changes needed.
| /** File existence checks configuration */ | ||
| files?: Array<{ | ||
| /** Key is the metric identifier, value is the file path */ | ||
| [metricId: string]: string; | ||
| }>; |
There was a problem hiding this comment.
I like this approach, however, I have an idea I'd like to share.
Since we already have the open_prs level, I propose implementing something similar for file checking. For example:
. . .
github?: {
files_check?: {
/** File existence checks configuration */
files?: Array<{
/** Key is the metric identifier, value is the file path */
[metricId: string]: string;
}>;
}
}
. . .I believe this is a better approach if we consider the architecture from the perspective of our GitHub DataSource. Essentially, we can leverage the GitHub integration to retrieve both 'open PRs' and 'file checks' through the same source
There was a problem hiding this comment.
Yes, I agree and the getProviderId and getMetricIds` could end up being like the following:
// Base provider ID (required by interface)
getProviderId(): string {
return 'github.files_check';
}
// All metric IDs this provider handles
getMetricIds(): string[] {
return this.fileConfigs.map(f => `github.files_check.${f.id}`);
}
2 participants
Hey, I just made a Pull Request!
Do not merge, POC to check potential changes for the file level metrics for the Scorecard plugin.
✔️ Checklist