search

GoogleCloudPlatform / prometheus-engine

Google Cloud Managed Service for Prometheus libraries and manifests.
https://g.co/cloud/managedprometheus
Apache License 2.0
195 stars 92 forks source link

Split types.go into multiple files #742

Closed TheSpiritXIII closed 10 months ago

TheSpiritXIII commented 10 months ago

Since the file was becoming unwieldy...

pintohutch commented 10 months ago

My vote would be to keep a single types.go with all the canonical structs and client generation tags, but move the individual functions to separate files.

TheSpiritXIII commented 10 months ago

Thanks for your insight, @pintohutch! Can you elaborate on some reasons why you think we should do it this way?

My reasons for not doing it this way are:

  1. controller-gen does not care what the file is called. It just scans for the annotations. Currently only po-docgen needs filenames, but this tool is deprecated and we should move off of it at some point.
  2. This is the same way other projects conventionally split up their types. For instance, a sample GitHub search shows that nearly everyone does a "_types.go" file suffix: https://github.com/search?q=%2Bkubebuilder%3Asubresource%3Astatus&type=code
  3. It feel unconventional to have functions in separate files.
TheSpiritXIII commented 10 months ago

Had an offline discussion with @pintohutch who okayed this change, as long as controller-generators still work (as part of #738).

TheSpiritXIII commented 10 months ago

fyi, renamed collection_types to pod_types, and created monitoring_types for common status objects that would be used by podmonitoring, nodemonitoring and probemonitoring!