-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: add githubgen tool #639
feat: add githubgen tool #639
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #639 +/- ##
==========================================
- Coverage 51.17% 47.13% -4.05%
==========================================
Files 51 56 +5
Lines 2882 3214 +332
==========================================
+ Hits 1475 1515 +40
- Misses 1260 1548 +288
- Partials 147 151 +4 ☔ View full report in Codecov by Sentry. |
const allowlistHeader = `# Code generated by githubgen. DO NOT EDIT. | ||
##################################################### | ||
# | ||
# List of components in OpenTelemetry Collector Contrib |
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.
hardcoded to contrib
const codeownersHeader = `# Code generated by githubgen. DO NOT EDIT. | ||
##################################################### | ||
# | ||
# List of codeowners for OpenTelemetry Collector Contrib |
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.
hardcoded, we might need to make that configurable.
# https://help.github.com/en/articles/about-code-owners | ||
# | ||
|
||
* @open-telemetry/collector-contrib-approvers |
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.
same
const distributionCodeownersHeader = ` | ||
##################################################### | ||
# | ||
# List of distribution maintainers for OpenTelemetry Collector Contrib |
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.
also hardcoded
githubgen/codeowners.go
Outdated
for stability := range m.Status.Stability { | ||
if stability == unmaintainedStatus { | ||
unmaintainedList += key + "/\n" | ||
unmaintainedCodeowners += fmt.Sprintf("%s/%s @open-telemetry/collector-contrib-approvers \n", key, strings.Repeat(" ", data.maxLength-len(key))) |
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.
hardcoded as well here.
githubgen/codeowners.go
Outdated
owners += " " | ||
owners += "@" + owner | ||
} | ||
codeowners += fmt.Sprintf("%s/%s @open-telemetry/collector-contrib-approvers%s\n", key, strings.Repeat(" ", data.maxLength-len(key)), owners) |
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.
hardcoded as well here
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 as a code introduction, we can add tests right now to check doc generation and make sure to add more config to override the default group assignment and project name.
@atoulme thanks for the review. I will add at least a sanity check unit test in this PR which will already need some slight refactoring and then do any further changes in follow-ups. |
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: odubajDT <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
89b4763
to
af6b14a
Compare
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
@pjanotti If you have the time, could you take a quick look at the failing CI check and try to help us figure out why it is failing? |
@mowies the issue can be solved by creating a |
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
@pjanotti wow thanks for the help, I don't think I would have found that by myself at all 😅 |
switch path[0] { | ||
case "receiver", "exporter", "extension", "processor", "connector": | ||
path[1] = strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(path[1], "internal"), "extension"), "exporter"), "connector"), "processor"), "receiver") | ||
path[len(path)-1] = strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(path[len(path)-1], "internal"), "extension"), "exporter"), "connector"), "processor"), "receiver") |
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.
This looks like it shouldn't be hardcoded, but configurable per repository?
…low ups are implemented Signed-off-by: Moritz Wiesinger <[email protected]>
Signed-off-by: Moritz Wiesinger <[email protected]>
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.
Approving based on the comment above
Fixes open-telemetry/opentelemetry-collector-contrib#36691