Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

tc package: Split enum into its own package#4293

Closed
zrhoffman wants to merge 14 commits intoapache:masterfrom
zrhoffman:move_enum
Closed

tc package: Split enum into its own package#4293
zrhoffman wants to merge 14 commits intoapache:masterfrom
zrhoffman:move_enum

Conversation

@zrhoffman
Copy link
Copy Markdown
Member

What does this PR (Pull Request) do?

  • This PR is not related to any Issue

This PR splits enum.go within the tc library into its own package. Reasons for doing this:

  • Looking at the line count of source files in the tc package (wc -l *.go | sort -g), enum.go is 727 lines, more lines than the next-longest file in the package (deliveryservices.go) by 122 lines. Excluding tests and vendor files, only 10 go sources (out of 584) in the whole project are longer than this.

  • The godoc for the tc package currently begins with this line:

    Package enum contains enumerations and strongly typed names.

Behavior should be unchanged by this PR, and the documentation contains no references to the tc library or enum.go, so changes to documentation are not necessary.

Because no code is added, existing tests are sufficient.

Which Traffic Control components are affected by this PR?

  • Traffic Monitor
  • Traffic Ops
  • Traffic Ops ORT
  • Experimental - Traffic Router Golang

What is the best way to verify this PR?

  • Run Traffic Ops Go unit tests
  • Run Traffic Ops API tests
  • Run Traffic Monitor unit tests
  • Run atstccfg unit tests
  • Run Traffic Router Golang's unit test

The following criteria are ALL met by this PR

  • I have explained why tests in this PR are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • An update to CHANGELOG.md is not necessary
  • This PR includes any and all required license headers
  • This PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

The contents of 9010b50, the package rename itself, is a result of using Goland's "refactor" feature to move enum.go.

@rob05c
Copy link
Copy Markdown
Member

rob05c commented Jan 14, 2020

Personally, I'd really prefer we kept this stuff in tc. It's used so much in so many places, it makes the code much terser to have e.g. tc.DeliveryServiceName everywhere than enum.DeliveryServiceName. The brevity was a big part of naming it tc.

I think it's also good to keep as much as we can in a library that indicates it's Traffic Control, so it's easy for people not intricately familiar with the code to immediately see it's ours and not a third-party. And we could do something like tcenum.DeliveryServiceName, but that's even more verbose.

enum.go is 727 lines

I wouldn't object to breaking that file into multiple files, though.

The godoc for the tc package currently begins with this line:
Package enum contains enumerations and strongly typed names.

That's a bug, we should definitely fix the GoDoc.

@zrhoffman
Copy link
Copy Markdown
Member Author

I can see the benefit of a short package name like tc. I'll submit an issue for the GoDoc and open a PR for breaking that file into multiple files later on.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants