[chore] generate issue templates with githubgen

[atoulme]

Remove a bash script, use go code instead.

[andrzej-stencel]

I appreciate moving from Bash to Golang. I just wish the Golang code was easier to read. This tool's code was already quite messy before and with this change, it's getting more messy and hard to follow.

From what I can understand, with this change running the githubgen tool has the following effect depending on the command line options:

$ ./githubgen -dependabot
generateDependabot

$ ./githubgen -issue-templates
generateIssueTemplates

$ ./githubgen
generateCodeownersFiles
generateDependabot

The options are inconsistent. Not passing the -dependabot option still executes the dependabot generator, but not passing the -issues-templates option does not execute the issue template generator. The code for those options is borderline hilarious 😄, including the unnecessary if statements:

	if !dependabotOnly && !issueTemplatesOnly {
		if err = generateCodeownersFiles(foldersList, allCodeowners, allowlistFilePath, components, maxLength); err != nil {
			return err
		}
	}

	if !issueTemplatesOnly {
		if err = generateDependabot(dependabotData); err != nil {
			return err
		}
		return nil
	}

	if err = generateIssueTemplates(foldersList); err != nil {
		return err
	}
	if issueTemplatesOnly {
		return nil
	}

	return nil

We could disregard this with "this is just a tool", but reading and reviewing this code is painful or amusing depending on reviewer's approach 😉, and definitely takes more time than it could. More importantly, messy code makes it easier to introduce bugs.

I propose a little rework of the tool for consistent behavior - either run all generators when no options are passed, or only run the generators explicitly specified with options, not running any generators if no option is specified. Having consistency in behavior should also make the code more comprehensible.

Here's example pseudo-code for the first approach:

func run(...) {
  ...
  var generatorsToRun []string
  if dependabotOnly {
    generatorsToRun := []string{generateDependabot}
  ) else if issueTemplatesOnly {
    generatorsToRun := []string{generateIssueTemplates}
  } else {
    generatorsToRun := []string{
      generateDependabot,
      generateIssueTemplates,
      generateCodeownersFiles,
    }
  }

  for generator := range generatorsToRun {
    err := generator()
    if err != nil {
      return err
    }
  } 
}

[atoulme]

Yes. It’s painful. Especially some of the weird contortions the code has to follow to match the existing rules. We can make things a bit nicer, however note that the generators all depend on the same one time parsing. There is some state to pass around.

[atoulme]

I am rereading your comment, and I agree - most of my code is the stuff of comedy, and I should not code past 7pm.
Give me a sec and I'll amend.

[atoulme]

@astencel-sumo did you want to review this PR again?

[andrzej-stencel]

Awesome! this is so much better to read! 😍 Thank you Antoine.

Just one fix is needed as the tool currently fails on the codeowners generator:

$ GITHUB_TOKEN=redacted githubgen codeowners
panic: strings: negative Repeat count

goroutine 1 [running]:
strings.Repeat({0x7e3000?, 0xc00026c5f0?}, 0x7e15b8?)
        /usr/local/go/src/strings/strings.go:554 +0x318
main.codeownersGenerator.generate({}, 0xc0001e6000)
        /home/andrzej/sumo/opentelemetry-collector-contrib/cmd/githubgen/codeowners.go:146 +0xa3d
main.run({0x7e15c8, 0x1}, {0x763965, 0x1b}, {0xc00002e2e0, 0x1, 0x58?})
        /home/andrzej/sumo/opentelemetry-collector-contrib/cmd/githubgen/main.go:174 +0x60a
main.main()
        /home/andrzej/sumo/opentelemetry-collector-contrib/cmd/githubgen/main.go:50 +0x3e9

[andrzej-stencel]

Thank you Antoine!

Here's the output I'm getting now:

$ GITHUB_TOKEN=redacted githubgen
The following modules were not added to Dependabot to keep within limits of 220
  - "/cmd/configschema"
  - "/cmd/githubgen"
  - "/cmd/mdatagen"
  - "/cmd/opampsupervisor"
  - "/extension/encoding/jaegerencodingextension"
  - "/extension/encoding/jsonlogencodingextension"
  - "/extension/encoding/textencodingextension"
  - "/extension/encoding/zipkinencodingextension"
  - "/extension/opampextension"
  - "/receiver/splunkenterprisereceiver"
2023/10/31 15:54:53 codeowners are not members: cparkins
$ echo $?
1

Seems that cperkins needs to be added to /cmd/githubgen/allowlist.txt if I'm not mistaken. Not sure if this is something that should be fixed as part of this change or separately.

[atoulme]

It needs to be fixed separately. Unfortunately, we still don't have a PAT to run this check as part of CI. Feel free to upvote open-telemetry/community#1659

[atoulme]

#28855 is open to fix this issue.

[andrzej-stencel]

I guess this can be merged now?

[mx-psi]

@atoulme can you confirm this is good to merge now?

[atoulme]

yes, good to merge, thanks!