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

Conversation

@ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented Aug 11, 2021

This PR modifies the packages

  • github.com/apache/trafficcontrol/lib/go-atscfg
  • github.com/apache/trafficcontrol/lib/go-log
  • github.com/apache/trafficcontrol/lib/go-rfc
  • github.com/apache/trafficcontrol/lib/go-util

This PR ensures that each modified package

  • Has a GoDoc comment for the package, and that comment is properly recognized
  • Has a GoDoc comment for every exported symbol
  • Uses complete sentences with proper capitalization and punctuation in GoDoc comments
  • Begins every GoDoc comment with either an optional article followed by the name of the documented symbol or the word "Package" followed by the name of the documented package.
  • Has no GoDoc comments of the form // SymbolName ... where ... is literal.

This PR also adds deprecation notices to some structures used only in deprecated API versions (2 and 3).

There should be only one actual code change from this PR - fixing a misspelling of "occurred" ("occured") -, but it does move imports around in affected files if necessary to match the pattern:

package packageName

// License header

import (
    "package names for import"
)

and possibly moves constants to logical groupings so that a single GoDoc can be written effectively for the group. In at least one case this involved moving a symbol to a different file (but not a different package).


Which Traffic Control components are affected by this PR?

  • Documentation (GoDoc comments)

What is the best way to verify this PR?

I used the command

golangci-lint run -c ./.golangci.yml 2>&1 | grep -E '^go-' | grep -v 'move short variable' | grep -v 'error strings should not be capitalized' | grep -v "don't use generic names such as" | grep -vE 'struct field `\w+` should be' | grep -v 'block ends with a' | grep -v 'errors.New(fmt.Sprintf(...))' | grep -vE 'receiver name \w+ should be consistent' | grep -v 'should not use basic type string as key in context.WithValue' | grep -vE 'var `\w+` should be `\w+`' | grep -v 'returns unexported type' | grep -v 'should omit 2nd value from range' | grep -v 'by other packages, and that stutters;' | grep -v "don't use ALL_CAPS in Go names; use CamelCase" | grep -v 'regexrevalidatedotconfig.go:36:7: const'

which should output nothing (but exit with a failure because grep found no matches), with the following in ./golangci.yml (under lib/):

Click to expand
run:
  concurrency: 4
  timeout: 5m
  issues-exit-code: 0 # TODO: once all enabled linters are satisfied
  tests: false
  skip-dirs-use-default: true

  output:
    format: colored-line-number
    print-issued-lines: true
    print-linter-name: true

issues:
  max-issues-per-linter: 0
  max-same-issues: 0
  exclude-use-default: false

  include:
    - EXC0002
    - EXC0011

linters:
  disable-all: true
  enable:
    - asciicheck
    - bodyclose
    - depguard
    - dogsled
    - durationcheck
    - gochecknoinits
    - godot
    - golint
    - gomodguard
    - govet
    - importas
    - makezero
    - misspell
    - nakedret
    - noctx
    - nolintlint
    - paralleltest
    - predeclared
    - sqlclosecheck
    - structcheck
    - testpackage
    - thelper
    - tparallel
    - typecheck
    - varcheck

Then to find things like // Foo ... I did

grep -RE '^// \w+ \.\.\.' .

... also within lib/, which should also output no lines (and exit failure b/c grep).

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added the tech debt rework due to choosing easy/limited solution label Aug 11, 2021
@ocket8888 ocket8888 requested a review from rob05c August 20, 2021 13:54
@ocket8888 ocket8888 mentioned this pull request Sep 10, 2021
@ocket8888 ocket8888 force-pushed the lib/godoc branch 3 times, most recently from 474926d to f9757b6 Compare February 25, 2022 23:41
@ocket8888 ocket8888 force-pushed the lib/godoc branch 3 times, most recently from 9fa64fd to 0972053 Compare July 25, 2022 15:24
@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Merging #6099 (903041e) into master (40f0b57) will increase coverage by 0.26%.
The diff coverage is 23.07%.

@@             Coverage Diff              @@
##             master    #6099      +/-   ##
============================================
+ Coverage     28.91%   29.18%   +0.26%     
============================================
  Files           600      518      -82     
  Lines         77353    75777    -1576     
  Branches         90      880     +790     
============================================
- Hits          22368    22114     -254     
+ Misses        52892    51786    -1106     
+ Partials       2093     1877     -216     
Flag Coverage Δ
golib_unit 53.57% <23.07%> (-0.02%) ⬇️
grove_unit 12.02% <ø> (ø)
t3c_generate_unit ?
t3c_unit 5.80% <ø> (-0.12%) ⬇️
traffic_monitor_unit 26.44% <ø> (ø)
traffic_ops_integration ?
traffic_ops_unit 21.67% <ø> (ø)
traffic_portal_v2 74.37% <ø> (?)
traffic_router_unit ?
traffic_stats_unit 10.78% <ø> (ø)
unit_tests 29.18% <23.07%> (+3.43%) ⬆️
v3 ?
v4 ?
v5 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/go-atscfg/astatsdotconfig.go 0.00% <ø> (ø)
lib/go-atscfg/atscfg.go 51.73% <ø> (ø)
lib/go-atscfg/atsdotrules.go 75.75% <ø> (ø)
lib/go-atscfg/bgfetchdotconfig.go 66.66% <ø> (ø)
lib/go-atscfg/cachedotconfig.go 43.30% <ø> (ø)
lib/go-atscfg/chkconfig.go 80.64% <ø> (ø)
lib/go-atscfg/dropqstringdotconfig.go 55.17% <ø> (ø)
lib/go-atscfg/facts.go 68.42% <ø> (ø)
lib/go-atscfg/headerrewritedotconfig.go 56.83% <ø> (ø)
lib/go-atscfg/hostingdotconfig.go 57.69% <ø> (ø)
... and 44 more

... and 318 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zrhoffman zrhoffman removed the request for review from rob05c September 26, 2023 23:45
@zrhoffman zrhoffman merged commit 9efc4d1 into apache:master Sep 26, 2023
jpappa200 pushed a commit to jpappa200/trafficcontrol that referenced this pull request Sep 27, 2023
* Add/Update/Correct GoDoc comments on cache-control-related symbols

* Formatting for caching symbol GoDoc comments

* Add/Update/Correct GoDoc comments for email-related structures

* Add/Update/Correct GoDoc comments on URL-related structures

* Add/Update/Correct GoDoc comments for logging-related structures

* Add package GoDoc documentation for lib/go-log/

* Add/Update/Correct GoDoc comments on "backoff"-related structures

* Add/Update/Correct GoDoc comment on encryption-related utilities

* Add/Update/Correct GoDoc comments on sting/error-joining-related utilities

* Add/Update/Correct GoDoc comments on IP address-related utilities

* Add/Update/Correct GoDoc comments on variable-typed numerics utilities

* Add/Update/Correct GoDoc comments on string slice utilities

* Add package GoDoc documentation for lib/go-util

* Add package GoDoc documentation for lib/go-atscfg

* Add/Update/Correct GoDoc comments for astats.config symbols

* Add/Update/Correct GoDoc comments for generic/widely used ATS configuration file-related structures

* Add/Update/Correct GoDoc comments for 50-ats.rules symbols

* Add/Update/Correct GoDoc comments for bg_fetch.config symbols

* Add/Update/Correct GoDoc comments for cache.config symbols

* Add/Update/Correct GoDoc comments for chkconfig symbols

* Add/Update/Correct GoDoc comments on drop_qstring.config symbols

* Add/Update/Correct GoDoc comments on 12M_facts symbols

* Add/Update/Correct GoDoc comments on hdr_rw_*.config symbols

* Add/Update/Correct GoDoc comments on hosting.config symbols

* Add/Update/Correct GoDoc comments on ip_allow.config symbols

* Add/Update/Correct GoDoc comments on ip_allow.yaml-related symbols

* Add/Update/Correct GoDoc comments on logging.config symbols

* Add/Update/Correct GoDoc comments on logging.yaml symbols

* Add/Update/Correct GoDoc comments for logs_xml.config symbols

* Add/Update/Correct GoDoc comments for configuration file "meta" symbols

* Add/Update/Correct GoDoc comments for package-related symbols

* Add/Update/Correct GoDoc comments for parent.config symbols

* Add/Update/Correct GoDoc comments for records.config symbols

* Add/Update/Correct GoDoc comments for regex_remap.config symbols

* Add/Update/Correct GoDoc comments for regex_revalidate.config symbols

* Add/Update/Correct GoDoc comments for remap.config symbols

* Add/Update/Correct GoDoc comments for "server" cache.config symbols

* Add/Update/Correct GoDoc comments for arbitrary file generation symbols

* Add/Update/Correct GoDoc comments for set_dscp.config symbols

* Add/Update/Correct GoDoc comments for sni.yaml symbols

* Add/Update/Correct GoDoc comments on ssl_multicert.config symbols

* Add/Update/Correct GoDoc comments on ssl_server_name.yaml symbols.

* Add/Update/Correct GoDoc comments for storage.config symbols

* Add/Update/Correct GoDoc comments for sysctl.conf symbols

* Add/Update/Correct GoDoc comments for uri_signing.config symbols

* Add/Update/Correct GoDoc comments for url_sig.config symbols

* Add/Update/Correct GoDoc comments for volume.config symbols

* Fix spelling errors

* Remove incorrect warning

* Update chkconfig GoDoc comments based on questions answered in the review

* Remove comments about removing constants that are only used tautologically

* Add/Update/Correct GoDoc comments for abstract parentage utilities

* Add/Update/Correct GoDoc comments for strategies.yaml symbols

* Add/Update/Correct GoDoc comments for symbols related to Delivery Service SSL/URI/URL keys

* Add/Update/Correct GoDoc comments for symbols related to servers

* Add/Update/Correct GoDoc comments for symbols related to Traffic Router

* Add/Update/Delete GoDoc comments for symbols related to emulated enumerated types

* Add/Update/Correct GoDoc comments for symbols related to content invalidation jobs

* Add/Update/Correct GoDoc comments for symbols related to users

* Go fmt
@ocket8888 ocket8888 deleted the lib/godoc branch September 27, 2023 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tech debt rework due to choosing easy/limited solution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants