From b2991f5c833c7aea5c8eae4dafcda38458b1366d Mon Sep 17 00:00:00 2001
From: Kate Osborn <k.osborn@f5.com>
Date: Wed, 24 May 2023 12:25:50 -0600
Subject: [PATCH 1/8] Add Go style guide

---
 docs/developer/go-style-guide.md | 458 +++++++++++++++++++++++++++++++
 1 file changed, 458 insertions(+)
 create mode 100644 docs/developer/go-style-guide.md

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
new file mode 100644
index 0000000000..d4d963918c
--- /dev/null
+++ b/docs/developer/go-style-guide.md
@@ -0,0 +1,458 @@
+# Go Style Guide
+
+Before diving into project-specific guidelines, please familiarize yourself with the following vetted best practices:
+
+- [Effective Go](https://go.dev/doc/effective_go)
+- [Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments)
+- [100 Go Mistakes](https://github.com/teivah/100-go-mistakes)
+
+Once you have a good grasp of these general best practices, you can then explore the project-specific guidelines below.
+These guidelines will often build upon the foundation set by the general best practices and provide additional
+recommendations tailored to the project's specific requirements and coding style.
+
+## Table of Contents
+
+1. [General Guidelines](#general-guidelines)
+2. [Error Handling](#error-handling)
+3. [Logging](#logging)
+4. [Concurrency](#concurrency)
+5. [Recommended/Situational Guidelines](#recommended--situational)
+
+## General Guidelines
+
+### Use the empty struct `struct{}` for sentinel values
+
+Empty structs as sentinels unambiguously signal an explicit lack of information. For example, use empty struct when for
+sets and for signaling via channels that don't require a message.
+
+DO:
+
+```go
+set := make(map[string]struct{}) // empty struct is nearly value-less
+
+signaller := make(chan struct{}, 0)
+signaller <- struct{}{} // no information but signal on delivery
+```
+
+DO NOT:
+
+```go
+set := make(map[string]bool) // is true/false meaningful? Is this a set?
+
+signaller := make(chan bool, 0)
+signaller <- false // is this a signal? is this an error?
+```
+
+### Consistent Line Breaks
+
+When breaking up a long function definition, call, or struct initialization, choose to break after each parameter,
+argument, or field.
+
+DO:
+
+```go
+func longFunctionDefinition(
+    paramX int,
+    paramY string,
+    paramZ bool,
+) (string, error){}
+
+// and 
+
+s := myStruct{
+    field1: 1,
+    field2: 2,
+    field3: 3,
+}
+```
+
+DO NOT:
+
+```go
+func longFunctionDefinition(paramX int, paramY string,
+    paramZ bool,
+) (string, error){}
+
+// or 
+
+func longFunctionDefinition(
+    paramX int, paramY string,
+    paramZ bool,
+) (string, error){}
+
+// or 
+s := myStruct{field1: 1, field2: 2,
+    field3: 3}
+
+```
+
+When constructing structs pass members during initialization.
+
+Example:
+
+```go
+cfg := foo.Config{
+    Site: "example.com",
+    Out: os.Stdout,
+    Dest: c.KeyPair{
+        Key: "style",
+        Value: "well formatted",
+    },
+}
+```
+
+### Do not copy sync entities
+
+`sync.Mutex` and `sync.Cond` MUST NOT be copied. By extension, structures holding an instance MUST NOT be copied, and
+structures which embed instances MUST NOT be copied.
+
+DO NOT embed sync entities. Pointers to `Mutex` and `Cond` are required for storage.
+
+### Construct slices with known capacity
+
+Whenever possible bounded slices should be constructed with a length of zero size, but known capacity.
+
+```go
+s := make([]string, 0, 32)
+```
+
+Growing a slice is an expensive deep copy operation. When the bounds of a slice can be calculated, pre-allocating the
+storage allows for append to assign a value without allocating new memory.
+
+### Accept interfaces and return structs
+
+Structures are expected to be return values from functions. If they satisfy an interface, any struct may be used in
+place of that interface. Interfaces will cause escape analysis and likely heap allocation when returned from a
+function - concrete instances (such as copies) may stay as stack memory.
+
+Returning interfaces from functions will hide the underlying structure type. This can lead to unintended growth of the
+interface type when methods are needed but unavailable, or an API change to return the structure later.
+
+Accepting interfaces as arguments ensures forward compatibility as API responsibilities grow. Structures as arguments
+will require additional functions or breaking API changes. Whereas interfaces inject behavior and may be replaced or
+modified without changing the signature.
+
+### Use contexts in a viral fashion
+
+Functions that accept a `context.Contex`t should pass the context or derive a subsidiary context to functions it calls.
+When designing libraries, subordinate functions (especially those asynchronous or expensive in nature) should accept
+`context.Context`.
+
+Public APIs SHOULD be built from inception to be context aware. ALL asynchronous public APIs MUST be built from
+inception to be context aware.
+
+### Do not use templates to replace interface types
+
+Use _templates_ to substitute for concrete types, use _interfaces_ to substitute for abstract behaviors.
+
+### Do not use booleans as function parameters
+
+A boolean can only express an on/off condition and will require a breaking change or new arguments in the future.
+Instead, pack functionality; for example, using integers instead of bools (maps and structs are also acceptable).
+
+DO NOT:
+
+```go
+func(bool userOn, bool groupOn, bool globalOn)
+```
+
+DO:
+
+```go
+func(uint permissions) // permissions := USER | GROUP | GLOBAL;  if permissions & USER then USER is set
+```
+
+### Use dependency injection to separate concerns
+
+Use dependency injection to separate high-level policy from low-level detail. Dependency injection promotes modular,
+testable, and maintainable code by reducing coupling and increasing flexibility.
+
+Creating dependencies couples ownership and lifetime while making tests difficult or impossible. Constructing
+dependencies adds side-effects which complicates testing.
+
+### Required arguments should be provided via parameters and optional arguments provided functionally or with structs
+
+DO NOT:
+
+```go 
+func(int required, int optional) {
+  if optional {...} 
+}
+```
+
+DO:
+
+```go
+type Option func (o *Object)
+
+func Optional(string optional) Option {
+  return func (o *Object) {
+    o.optional = optional
+  } 
+}
+
+func (int required, ...Options) {
+  for o := range Options {
+    o(self)
+  } 
+}
+```
+
+## Error Handling
+
+### Do not filter context when returning errors
+
+Preserve error context by wrapping errors as the stack unwinds. Utilize native error wrapping with `fmt.Errorf` and
+the `%w` verb to wrap errors. Wrapped errors offer a transparent view to end users. For a practical example, refer to
+this runnable code snippet: [Go Playground Example](https://go.dev/play/p/f9EaJDB5JUO). When required, you can identify
+inner wrapped errors using native APIs such as `As`, `Is`, and `Unwrap`.
+See [Working with errors](https://go.dev/blog/go1.13-errors) for more information on error wrapping.
+
+### Only handle errors once
+
+DO NOT log an error, then subsequently return that error. This creates the potential for multiple error reports and
+conflicting information to users.
+
+DO NOT:
+
+```go
+func badAtStuff(noData string) error {
+  if len(noData) == 0 {
+    fmt.Printf("Received no data")
+  }
+
+  return errors.New("received no data")
+}
+```
+
+DO
+
+```go
+func badAtStuff(noData string) error {
+  if len(noData) == 0 {
+    return errors.New("received no data")
+  }
+  ...
+}
+```
+
+A leader of the golang community once said:
+> Lastly, I want to mention that you should only handle errors once. Handling an error means inspecting the
+> error value, and making a decision. If you make less than one decision, you’re ignoring the error...But making
+> more than one decision in response to a single error is also problematic. - Dave Cheney
+
+### Libraries should return errors for callers to handle
+
+Asynchronous libraries should communicate via channels or callbacks. Only then should they log unhandled errors.
+
+Example:
+
+```go
+func onError(err error) {
+    // got an asynchronous error 
+}
+
+func ReadAsync(r io.Reader, onError) {
+  err := r()
+  if err != nil {
+    onError(err)
+  } 
+}
+
+go ReadAsync(reader, onError)
+
+// OR errs := make(chan error)
+
+func ReadAsync(r io.Reader, errs chan<- error) {
+  err := r()
+  if err != nil {
+    // put error on errs channel, but don't block forever.
+  }
+}
+```
+
+### Callers should handle errors and pass them up the stack for notification
+
+Callers should handle errors that occur within the functions they call. This allows them to handle errors according to
+their specific requirements. However, if callers are unable to handle the error or need to provide additional context,
+they can add context to the error and pass it up the stack for notification.
+
+Example:
+
+```go
+func readFile(filename string) ([]byte, error) {
+  file, err := os.Open(filename)
+  if err != nil {
+    return nil, fmt.Errorf("failed to open file: %w", err)
+  }
+  defer file.Close()
+  
+  data, err := ioutil.ReadAll(file)
+  if err != nil {
+    return nil, fmt.Errorf("failed to read file: %w", err)
+  }
+  
+  return data, nil
+}
+
+func processFile(filename string) error {
+  data, err := readFile(filename)
+  if err != nil {
+    return fmt.Errorf("failed to process file: %w", err)
+  }
+  // Process the file data here
+  return nil
+}
+
+func main() {
+  filename := "example.txt"
+  err := processFile(filename)
+  if err != nil {
+    fmt.Printf("Error processing file: %v\n", err) // caller handles the error
+  }
+}
+```
+
+## Logging
+
+> **Note**
+> This section is a work in progress.
+
+NKG uses [logr](https://pkg.go.dev/github.com/go-logr/logr) for its logging library. Each component that logs should
+inherit their logger from the main process using
+the [`WithName`](https://pkg.go.dev/github.com/go-logr/logr#hdr-Logger_Names) function.
+
+## Concurrency
+
+Below are some general guidelines to follow for writing concurrent code:
+
+- **Don't assume that a concurrent solution will be faster than an iterative one**: Benchmark the iterative and
+  concurrent solutions before deciding which one to use.
+- **Don't add synchronization unless strictly necessary**: Synchronization primitives -- such as mutexes -- are costly.
+  Only use them when the code is accessed by multiple goroutines concurrently.
+- **Document when exported code is not concurrent-safe**: If an exported interface, object, or function is not
+  reentrant, you _must_ document that in the comments. Make it clear and obvious.
+- **Don't leak goroutines**: Goroutines are not garbage collected by the runtime, so every goroutine you start must also
+  be cleaned up. Here's a couple of related principles:
+  - "If a goroutine is responsible for creating a goroutine, it is also responsible for ensuring it can stop the
+    goroutine." -- [Concurrency in Go][cig]
+  - "Before you start a goroutine, always know when, and how, it will stop." -- [Concurrency Made Easy][cheney].
+- **Blocking operations within a goroutine must be preemptable**: This allows goroutines to be cancelled and prevents
+  goroutine leaks.
+- **Leverage contexts**: Contexts allow you to enforce deadlines and send cancellation signals to multiple goroutines.
+- **Avoid buffered channels**:  Use unbuffered channels unless there is a very good reason for using a buffered channel.
+  Unbuffered channels provide strong synchronization guarantees. Buffered channels are asynchronous and will not block
+  unless the channel is full. Buffered channels can also be slower than unbuffered channels.
+- **Protect maps and slices**: Maps and slices cannot be accessed concurrently without locking. Doing so can lead to
+  data races.
+- **Never copy sync types**: see [above section](#do-not-copy-sync-entities).
+- **Choose primitives or channels based on use case**: In general, the Go language writers tell us to prefer channels
+  and communication for synchronization over primitives in the sync package such as mutexes and wait groups. "Do not
+  communicate by sharing memory. Instead, share memory by communicating". However, in practice, there are some cases
+  where sync primitives are the better choice. For example, you should use primitives if you are working with a
+  performance-critical section, or protecting the internal state of a struct. However, you should use channels if you
+  are transferring ownership of data (e.g. producer/consumer) or trying to coordinate multiple pieces of logic. 
+- **When possible, write code that is implicitly concurrent-safe**: Code that is implicitly concurrent-safe can be
+  safely accessed by multiple goroutines concurrently without any synchronization. For example, immutable data is
+  implicitly concurrent-safe. Concurrent processes can operate on the data, but they can't modify it. Another example is
+  lexical confinement. From [Concurrency in Go][cig]: "Lexical confinement involves using lexical scope to expose only
+  the correct data and concurrency primitives for multiple concurrent processes to use. It makes it impossible to do the
+  wrong thing." Code that is implicitly concurrent-safe is typically more performant and easier for developers to
+  understand.
+- **Leverage errgroup**: The package [errgroup]((https://pkg.go.dev/golang.org/x/sync/errgroup)) helps synchronize a
+  group of goroutines that return errors.
+- **Release locks and semaphores in the reverse order you acquire them**: This will prevent lock inversion and
+  deadlocks.
+- **Close channels to signal receivers NOT to free resources**: Channels do not need to be closed to free resources.
+  Only close channels as a means to signal the channel's receivers that the channel is done accepting new data.
+
+Sources and Recommend Reading:
+
+- [Concurrency in Go][cig]
+- [Concurrency Made Easy][cheney]
+- [Channel Axioms](https://dave.cheney.net/2014/03/19/channel-axioms)
+- [Concurrency is not parallelism](https://go.dev/blog/waza-talk)
+- [Pipelines and cancellation](https://go.dev/blog/pipelines)
+- [100 Go Mistakes; Chapters 8 & 9](https://github.com/teivah/100-go-mistakes)
+
+[cig]:https://learning.oreilly.com/library/view/concurrency-in-go/9781491941294/
+
+[cheney]: https://dave.cheney.net/paste/concurrency-made-easy.pdf
+
+## Recommended / Situational
+
+These recommendations are generally related to performance and efficiency but will not be appropriate for all paradigms.
+
+### Use golang benchmark tests and pprof tools for profiling and identifying hot spots
+
+The `-gcflags '-m'` can be used to analyze escape analysis and estimate logging costs.
+
+### Reduce the number of stored pointers. Structures should store instances whenever possible.
+
+DO NOT use pointers to avoid copying. Pass by value. Ancillary benefit is reduction of nil checks. Fewer pointers helps
+garbage collection and can indicate memory regions that can be skipped. It reduces de-referencing and bounds checking in
+the VM. Keep as much on the stack as possible (see caveat: [accept interfaces](#accept-interfaces-and-return-structs) -
+forward compatibility and flexibility concerns outweigh costs of heap allocation)
+
+FAVOR:
+
+```go
+type Object struct{
+    subobject SubObject
+}
+
+func New() Object {
+  return Object{
+    subobject: SubObject{},
+  }
+}
+```
+
+DISFAVOR:
+
+```go
+type Object struct{
+    subobject *SubObject
+}
+
+func New() *Object {
+  return &Object{
+    subobject: &SubObject{},
+  } 
+}
+```
+
+### Pass pointers down the stack not up
+
+Pointers can be passed down the stack without causing heap allocations. Passing pointers up the stack will cause heap
+allocations.
+
+```text
+-> initialize struct A -> func_1(&A)
+-> func_2(&A)
+```
+
+`A` can be passed as a pointer as it's passed down the stack.
+
+Returning pointers can cause heap allocations and should be avoided.
+
+DO NOT:
+
+```go
+func(s string) *string {
+  s := s + "more strings"
+  return &s // this will move to heap 
+}
+```
+
+### Using interface types will cause unavoidable heap allocations
+
+Frequently created, short-lived instances will cause heap and garbage collection pressure. Using a `sync.Pool` to store
+and retrieve structures can improve performance.
+
+Before adopting a `sync.Pool` analyze the frequency of creation and duration of lifetime. Objects created frequency for
+short periods of time will benefit the most.
+
+For example, channels that send and receive signals using interfaces; these are often only referenced for the duration
+of the event and can be recycled once the signal is received and processed.
+ Guidelines

From a413cd0de1f71efa3d4572da9f5a9ab8cc5a1b19 Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Thu, 25 May 2023 11:19:39 -0600
Subject: [PATCH 2/8] Fix Context typo

Co-authored-by: Ciara Stacke <18287516+ciarams87@users.noreply.github.com>
---
 docs/developer/go-style-guide.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
index d4d963918c..847434fb7a 100644
--- a/docs/developer/go-style-guide.md
+++ b/docs/developer/go-style-guide.md
@@ -134,7 +134,7 @@ modified without changing the signature.
 
 ### Use contexts in a viral fashion
 
-Functions that accept a `context.Contex`t should pass the context or derive a subsidiary context to functions it calls.
+Functions that accept a `context.Context` should pass the context or derive a subsidiary context to functions it calls.
 When designing libraries, subordinate functions (especially those asynchronous or expensive in nature) should accept
 `context.Context`.
 

From bd9892da964aa6ceeb2e8f956a4c804aeaa308df Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Thu, 25 May 2023 13:04:08 -0600
Subject: [PATCH 3/8] Fix typo

Co-authored-by: Saylor Berman <s.berman@f5.com>
---
 docs/developer/go-style-guide.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
index 847434fb7a..7f0443182a 100644
--- a/docs/developer/go-style-guide.md
+++ b/docs/developer/go-style-guide.md
@@ -22,7 +22,7 @@ recommendations tailored to the project's specific requirements and coding style
 
 ### Use the empty struct `struct{}` for sentinel values
 
-Empty structs as sentinels unambiguously signal an explicit lack of information. For example, use empty struct when for
+Empty structs as sentinels unambiguously signal an explicit lack of information. For example, use empty struct for
 sets and for signaling via channels that don't require a message.
 
 DO:

From d43f6c31e85eebfef1c1ae223ccad6db9ba64bb8 Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Thu, 25 May 2023 13:07:08 -0600
Subject: [PATCH 4/8] Spell Recommended correctly

Co-authored-by: Saylor Berman <s.berman@f5.com>
---
 docs/developer/go-style-guide.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
index 7f0443182a..38a9018a60 100644
--- a/docs/developer/go-style-guide.md
+++ b/docs/developer/go-style-guide.md
@@ -366,7 +366,7 @@ Below are some general guidelines to follow for writing concurrent code:
 - **Close channels to signal receivers NOT to free resources**: Channels do not need to be closed to free resources.
   Only close channels as a means to signal the channel's receivers that the channel is done accepting new data.
 
-Sources and Recommend Reading:
+Sources and Recommended Reading:
 
 - [Concurrency in Go][cig]
 - [Concurrency Made Easy][cheney]

From 41dd0235b46a51299278d78aa6d844900615da12 Mon Sep 17 00:00:00 2001
From: Kate Osborn <k.osborn@f5.com>
Date: Thu, 25 May 2023 13:08:13 -0600
Subject: [PATCH 5/8] Remove hanging guidelines

---
 docs/developer/go-style-guide.md | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
index 38a9018a60..a855d31aed 100644
--- a/docs/developer/go-style-guide.md
+++ b/docs/developer/go-style-guide.md
@@ -455,4 +455,3 @@ short periods of time will benefit the most.
 
 For example, channels that send and receive signals using interfaces; these are often only referenced for the duration
 of the event and can be recycled once the signal is received and processed.
- Guidelines

From 07d0027973c7cfeaadd57bbcc179d8fb708c92c0 Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Tue, 30 May 2023 14:48:05 -0600
Subject: [PATCH 6/8] Add caveat to protecting maps and slices

Co-authored-by: Michael Pleshakov <pleshakov@users.noreply.github.com>
---
 docs/developer/go-style-guide.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
index a855d31aed..4f9bc8ed35 100644
--- a/docs/developer/go-style-guide.md
+++ b/docs/developer/go-style-guide.md
@@ -343,7 +343,7 @@ Below are some general guidelines to follow for writing concurrent code:
 - **Avoid buffered channels**:  Use unbuffered channels unless there is a very good reason for using a buffered channel.
   Unbuffered channels provide strong synchronization guarantees. Buffered channels are asynchronous and will not block
   unless the channel is full. Buffered channels can also be slower than unbuffered channels.
-- **Protect maps and slices**: Maps and slices cannot be accessed concurrently without locking. Doing so can lead to
+- **Protect maps and slices**: Maps and slices cannot be accessed concurrently (when at least one goroutine is writing) without locking. Doing so can lead to
   data races.
 - **Never copy sync types**: see [above section](#do-not-copy-sync-entities).
 - **Choose primitives or channels based on use case**: In general, the Go language writers tell us to prefer channels

From 4a098949d9a9b75b99bfc4c3c1d2a5261ffae46b Mon Sep 17 00:00:00 2001
From: Kate Osborn <k.osborn@f5.com>
Date: Tue, 30 May 2023 15:10:55 -0600
Subject: [PATCH 7/8] Add section on panic

---
 docs/developer/go-style-guide.md | 177 +++++++++++++++++--------------
 1 file changed, 96 insertions(+), 81 deletions(-)

diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md
index 4f9bc8ed35..866023307e 100644
--- a/docs/developer/go-style-guide.md
+++ b/docs/developer/go-style-guide.md
@@ -22,8 +22,8 @@ recommendations tailored to the project's specific requirements and coding style
 
 ### Use the empty struct `struct{}` for sentinel values
 
-Empty structs as sentinels unambiguously signal an explicit lack of information. For example, use empty struct for
-sets and for signaling via channels that don't require a message.
+Empty structs as sentinels unambiguously signal an explicit lack of information. For example, use empty struct for sets
+and for signaling via channels that don't require a message.
 
 DO:
 
@@ -52,17 +52,17 @@ DO:
 
 ```go
 func longFunctionDefinition(
-    paramX int,
-    paramY string,
-    paramZ bool,
+paramX int,
+paramY string,
+paramZ bool,
 ) (string, error){}
 
 // and 
 
 s := myStruct{
-    field1: 1,
-    field2: 2,
-    field3: 3,
+field1: 1,
+field2: 2,
+field3: 3,
 }
 ```
 
@@ -70,19 +70,19 @@ DO NOT:
 
 ```go
 func longFunctionDefinition(paramX int, paramY string,
-    paramZ bool,
+paramZ bool,
 ) (string, error){}
 
 // or 
 
 func longFunctionDefinition(
-    paramX int, paramY string,
-    paramZ bool,
+paramX int, paramY string,
+paramZ bool,
 ) (string, error){}
 
 // or 
 s := myStruct{field1: 1, field2: 2,
-    field3: 3}
+field3: 3}
 
 ```
 
@@ -92,12 +92,12 @@ Example:
 
 ```go
 cfg := foo.Config{
-    Site: "example.com",
-    Out: os.Stdout,
-    Dest: c.KeyPair{
-        Key: "style",
-        Value: "well formatted",
-    },
+Site: "example.com",
+Out: os.Stdout,
+Dest: c.KeyPair{
+Key: "style",
+Value: "well formatted",
+},
 }
 ```
 
@@ -176,7 +176,7 @@ DO NOT:
 
 ```go 
 func(int required, int optional) {
-  if optional {...} 
+if optional {...} 
 }
 ```
 
@@ -186,15 +186,15 @@ DO:
 type Option func (o *Object)
 
 func Optional(string optional) Option {
-  return func (o *Object) {
-    o.optional = optional
-  } 
+return func (o *Object) {
+o.optional = optional
+} 
 }
 
 func (int required, ...Options) {
-  for o := range Options {
-    o(self)
-  } 
+for o := range Options {
+o(self)
+} 
 }
 ```
 
@@ -217,11 +217,11 @@ DO NOT:
 
 ```go
 func badAtStuff(noData string) error {
-  if len(noData) == 0 {
-    fmt.Printf("Received no data")
-  }
+if len(noData) == 0 {
+fmt.Printf("Received no data")
+}
 
-  return errors.New("received no data")
+return errors.New("received no data")
 }
 ```
 
@@ -229,10 +229,10 @@ DO
 
 ```go
 func badAtStuff(noData string) error {
-  if len(noData) == 0 {
-    return errors.New("received no data")
-  }
-  ...
+if len(noData) == 0 {
+return errors.New("received no data")
+}
+...
 }
 ```
 
@@ -249,14 +249,14 @@ Example:
 
 ```go
 func onError(err error) {
-    // got an asynchronous error 
+// got an asynchronous error 
 }
 
 func ReadAsync(r io.Reader, onError) {
-  err := r()
-  if err != nil {
-    onError(err)
-  } 
+err := r()
+if err != nil {
+onError(err)
+} 
 }
 
 go ReadAsync(reader, onError)
@@ -264,10 +264,10 @@ go ReadAsync(reader, onError)
 // OR errs := make(chan error)
 
 func ReadAsync(r io.Reader, errs chan<- error) {
-  err := r()
-  if err != nil {
-    // put error on errs channel, but don't block forever.
-  }
+err := r()
+if err != nil {
+// put error on errs channel, but don't block forever.
+}
 }
 ```
 
@@ -281,36 +281,51 @@ Example:
 
 ```go
 func readFile(filename string) ([]byte, error) {
-  file, err := os.Open(filename)
-  if err != nil {
-    return nil, fmt.Errorf("failed to open file: %w", err)
-  }
-  defer file.Close()
-  
-  data, err := ioutil.ReadAll(file)
-  if err != nil {
-    return nil, fmt.Errorf("failed to read file: %w", err)
-  }
-  
-  return data, nil
+file, err := os.Open(filename)
+if err != nil {
+return nil, fmt.Errorf("failed to open file: %w", err)
+}
+defer file.Close()
+
+data, err := ioutil.ReadAll(file)
+if err != nil {
+return nil, fmt.Errorf("failed to read file: %w", err)
+}
+
+return data, nil
 }
 
 func processFile(filename string) error {
-  data, err := readFile(filename)
-  if err != nil {
-    return fmt.Errorf("failed to process file: %w", err)
-  }
-  // Process the file data here
-  return nil
+data, err := readFile(filename)
+if err != nil {
+return fmt.Errorf("failed to process file: %w", err)
+}
+// Process the file data here
+return nil
 }
 
 func main() {
-  filename := "example.txt"
-  err := processFile(filename)
-  if err != nil {
-    fmt.Printf("Error processing file: %v\n", err) // caller handles the error
-  }
+filename := "example.txt"
+err := processFile(filename)
+if err != nil {
+fmt.Printf("Error processing file: %v\n", err) // caller handles the error
 }
+}
+```
+
+### Use panics for unrecoverable errors or programming errors
+
+Panics should be used in the following cases:
+
+1. Unrecoverable errors. An unrecoverable error is when NKG cannot continue running or its behavior or internal state
+   cannot be guaranteed. One example of this is if an error occurs when adding the Kubernetes API types to the Scheme,
+   or if an error occurs when marking a CLI flag as required.
+2. Programming errors. A programming error is an error that is only possible if there was a programming mistake. For
+   example, if the wrong type is passed or a go template is passed an invalid value.
+
+When using panics, pass an error as the argument. For example:
+```go
+panic(fmt.Errorf("unknown event type %T", e))
 ```
 
 ## Logging
@@ -334,24 +349,24 @@ Below are some general guidelines to follow for writing concurrent code:
   reentrant, you _must_ document that in the comments. Make it clear and obvious.
 - **Don't leak goroutines**: Goroutines are not garbage collected by the runtime, so every goroutine you start must also
   be cleaned up. Here's a couple of related principles:
-  - "If a goroutine is responsible for creating a goroutine, it is also responsible for ensuring it can stop the
-    goroutine." -- [Concurrency in Go][cig]
-  - "Before you start a goroutine, always know when, and how, it will stop." -- [Concurrency Made Easy][cheney].
+    - "If a goroutine is responsible for creating a goroutine, it is also responsible for ensuring it can stop the
+      goroutine." -- [Concurrency in Go][cig]
+    - "Before you start a goroutine, always know when, and how, it will stop." -- [Concurrency Made Easy][cheney].
 - **Blocking operations within a goroutine must be preemptable**: This allows goroutines to be cancelled and prevents
   goroutine leaks.
 - **Leverage contexts**: Contexts allow you to enforce deadlines and send cancellation signals to multiple goroutines.
 - **Avoid buffered channels**:  Use unbuffered channels unless there is a very good reason for using a buffered channel.
   Unbuffered channels provide strong synchronization guarantees. Buffered channels are asynchronous and will not block
   unless the channel is full. Buffered channels can also be slower than unbuffered channels.
-- **Protect maps and slices**: Maps and slices cannot be accessed concurrently (when at least one goroutine is writing) without locking. Doing so can lead to
-  data races.
+- **Protect maps and slices**: Maps and slices cannot be accessed concurrently (when at least one goroutine is writing)
+  without locking. Doing so can lead to data races.
 - **Never copy sync types**: see [above section](#do-not-copy-sync-entities).
 - **Choose primitives or channels based on use case**: In general, the Go language writers tell us to prefer channels
   and communication for synchronization over primitives in the sync package such as mutexes and wait groups. "Do not
   communicate by sharing memory. Instead, share memory by communicating". However, in practice, there are some cases
   where sync primitives are the better choice. For example, you should use primitives if you are working with a
   performance-critical section, or protecting the internal state of a struct. However, you should use channels if you
-  are transferring ownership of data (e.g. producer/consumer) or trying to coordinate multiple pieces of logic. 
+  are transferring ownership of data (e.g. producer/consumer) or trying to coordinate multiple pieces of logic.
 - **When possible, write code that is implicitly concurrent-safe**: Code that is implicitly concurrent-safe can be
   safely accessed by multiple goroutines concurrently without any synchronization. For example, immutable data is
   implicitly concurrent-safe. Concurrent processes can operate on the data, but they can't modify it. Another example is
@@ -398,13 +413,13 @@ FAVOR:
 
 ```go
 type Object struct{
-    subobject SubObject
+subobject SubObject
 }
 
 func New() Object {
-  return Object{
-    subobject: SubObject{},
-  }
+return Object{
+subobject: SubObject{},
+}
 }
 ```
 
@@ -412,13 +427,13 @@ DISFAVOR:
 
 ```go
 type Object struct{
-    subobject *SubObject
+subobject *SubObject
 }
 
 func New() *Object {
-  return &Object{
-    subobject: &SubObject{},
-  } 
+return &Object{
+subobject: &SubObject{},
+} 
 }
 ```
 
@@ -440,8 +455,8 @@ DO NOT:
 
 ```go
 func(s string) *string {
-  s := s + "more strings"
-  return &s // this will move to heap 
+s := s + "more strings"
+return &s // this will move to heap 
 }
 ```
 

From 8afdf893d241884a951d49eb4e17c7857eaba8db Mon Sep 17 00:00:00 2001
From: Kate Osborn <k.osborn@f5.com>
Date: Tue, 30 May 2023 15:18:15 -0600
Subject: [PATCH 8/8] Add links to code style guide

---
 CONTRIBUTING.md                          | 39 ++++--------------------
 docs/developer/implementing-a-feature.md | 22 ++++++-------
 2 files changed, 17 insertions(+), 44 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index a1c797ea41..244ad9d4bd 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -14,11 +14,6 @@ considering contributing!
 * [Issues and Discussions](#issues-and-discussions)
 * [Development Guide](#development-guide)
 
-[Style Guides](#style-guides)
-
-* [Git Style Guide](#git-style-guide)
-* [Go Style Guide](#go-style-guide)
-
 [Code of Conduct](CODE_OF_CONDUCT.md)
 
 [Contributor License Agreement](#contributor-license-agreement)
@@ -74,8 +69,8 @@ template.
 #### Issue lifecycle
 
 When an issue or PR is created, it will be triaged by the maintainers and assigned a label to indicate the type of issue
-it is (bug, feature request, etc) and to determine the milestone. See the [Issue Lifecycle](/ISSUE_LIFECYCLE.md) document
-for more information.
+it is (bug, feature request, etc) and to determine the milestone. See the [Issue Lifecycle](/ISSUE_LIFECYCLE.md)
+document for more information.
 
 ### Development Guide
 
@@ -85,36 +80,14 @@ Before beginning development, familiarize yourself with the following documents:
   your development environment and executing tasks required when submitting a pull request.
 - [Branching and Workflow](/docs/developer/branching-and-workflow.md): This document outlines the project's specific
   branching and workflow practices, including instructions on how to name a branch.
-- [Implement a Feature](/docs/developer/implementing-a-feature.md): A step-by-step guide on how to implement a feature or
-  bug.
+- [Implement a Feature](/docs/developer/implementing-a-feature.md): A step-by-step guide on how to implement a feature
+  or bug.
 - [Testing](/docs/developer/testing.md): The project's testing guidelines, includes both unit testing and manual testing
   procedures. This document explains how to write and run unit tests, and how to manually verify changes.
 - [Pull Request Guidelines](/docs/developer/pull-request.md): A guide for both pull request submitters and reviewers,
   outlining guidelines and best practices to ensure smooth and efficient pull request processes.
-
-## Style Guides
-
-### Git Style Guide
-
-* Keep a clean, concise and meaningful git commit history on your branch, rebasing locally and squashing before
-  submitting a PR
-* Follow the guidelines of writing a good commit message as described [here](https://chris.beams.io/posts/git-commit/)
-  and summarized in the next few points
-    * In the subject line, use the present tense ("Add feature" not "Added feature")
-    * In the subject line, use the imperative mood ("Move cursor to..." not "Moves cursor to...")
-    * Limit the subject line to 72 characters or less
-    * Reference issues and pull requests liberally after the subject line
-    * Add more detailed description in the body of the git message (`git commit -a` to give you more space and time in
-      your text editor to write a good message instead of `git commit -am`)
-
-### Go Style Guide
-
-* Run `gofmt` over your code to automatically resolve a lot of style issues. Most editors support this running
-  automatically when saving a code file.
-* Run `go lint` and `go vet` on your code too to catch any other issues.
-* Follow this guide on some good practice and idioms for Go -  https://github.com/golang/go/wiki/CodeReviewComments
-* To check for extra issues, install [golangci-lint](https://github.com/golangci/golangci-lint) and run `make lint`
-  or `golangci-lint run`
+- [Go Style Guide](/docs/developer/go-style-guide.md) A coding style guide for Go. Contains best practices and
+  conventions to follow when writing Go code for the project. 
 
 ## Contributor License Agreement
 
diff --git a/docs/developer/implementing-a-feature.md b/docs/developer/implementing-a-feature.md
index c797737cf3..2a1c7a4776 100644
--- a/docs/developer/implementing-a-feature.md
+++ b/docs/developer/implementing-a-feature.md
@@ -17,19 +17,21 @@ practices to ensure a successful feature development process.
    the [branching and workflow](/docs/developer/branching-and-workflow.md) documentation.
 4. **Branch**: Create a branch following
    the [naming conventions](/docs/developer/branching-and-workflow.md#branch-naming-conventions).
-5. **Make changes**: Make the necessary changes for the feature.
-6. **Consider opening a draft PR**: If your feature involves substantial architecture changes, or you would like to
+5. **Review style guide** Review the [Go style guide](/docs/developer/go-style-guide.md) to familiarize yourself with
+   the project's coding practices.
+6. **Make changes**: Make the necessary changes for the feature.
+7. **Consider opening a draft PR**: If your feature involves substantial architecture changes, or you would like to
    receive early feedback, consider opening a draft PR and requesting reviews from the maintainers. Draft PRs are an
    effective means to solicit feedback and ensure that major architectural changes align with the project's goals and
    standards.
-7. **Add or update unit tests** All features **must** be accompanied by unit tests that verify the functionality. Make
+8. **Add or update unit tests** All features **must** be accompanied by unit tests that verify the functionality. Make
    sure to thoroughly test the different scenarios and edge cases related to the feature to ensure robustness and
    reliability. Additionally, open the code coverage report to ensure that the code you added has sufficient test
    coverage. For instructions on writing and running unit tests, refer to
    the [testing](/docs/developer/testing.md#unit-test-guidelines) documentation.
-8. **Manually verify your changes**: Refer to the [manual testing](/docs/developer/testing.md#manual-testing) section of
+9. **Manually verify your changes**: Refer to the [manual testing](/docs/developer/testing.md#manual-testing) section of
    the testing documentation for instructions on how to manually test your changes.
-9. **Update any relevant documentation**: Here are some guidelines for updating documentation:
+10. **Update any relevant documentation**: Here are some guidelines for updating documentation:
     - **Gateway API Feature**: If you are implementing a Gateway API feature, make sure to update
       the [Gateway API Compatibility](/docs/gateway-api-compatibility.md) documentation.
     - **New Use Case:** If your feature introduces a new use case, add an example of how to use it in
@@ -41,22 +43,20 @@ practices to ensure a successful feature development process.
     - **Other Documentation Updates**: For any other changes that affect the behavior, usage, or configuration of NKG,
       review the existing documentation and update it as necessary. Ensure that the documentation remains accurate and
       up to date with the latest changes.
-10. **Lint code**: See the [run the linter](/docs/developer/quickstart.md#run-the-linter) section of the quickstart
+11. **Lint code**: See the [run the linter](/docs/developer/quickstart.md#run-the-linter) section of the quickstart
     guide for instructions.
-11. **Open pull request**: Open a pull request targeting the `main` branch of
+12. **Open pull request**: Open a pull request targeting the `main` branch of
     the [nginx-kubernetes-gateway](https://github.com/nginxinc/nginx-kubernetes-gateway/tree/main) repository. The
     entire `nginx-kubernetes-gateway` group will be automatically requested for review. If you have a specific or
     different reviewer in mind, you can request them as well. Refer to
     the [pull request](/docs/developer/pull-request.md) documentation for expectations and guidelines.
-12. **Obtain the necessary approvals**: Work with code reviewers to maintain the required number of approvals.
-13. **Squash and merge**: Squash your commits locally, or use the GitHub UI to squash and merge. Only one commit per
+13. **Obtain the necessary approvals**: Work with code reviewers to maintain the required number of approvals.
+14. **Squash and merge**: Squash your commits locally, or use the GitHub UI to squash and merge. Only one commit per
     pull request should be merged. Make sure the first line of the final commit message includes the pull request
     number. For example, Fix supported gateway conditions in compatibility doc (#674).
     > **Note**:
     When you squash commits, make sure to not include any commit messages related to the code review
     (for example, Fixed a typo). If you changed the code as a result of the code review in way that the original commit message no longer describes it well, make sure to update the message.
-     
-
 
 ## Fixing a Bug