refactor: implement docker-native vt ps and remove bolt-db state#161
refactor: implement docker-native vt ps and remove bolt-db state#161
Conversation
WalkthroughRemoved persistent state and disk storage; providers now report active deployments at runtime via a new Provider.List() API and DockerCompose provider changes. Constructor signatures and wiring were updated to drop state manager dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as "CLI (ps)"
participant App as "App / Registry"
participant Provider as "Provider (DockerCompose)"
participant Docker as "Docker Engine"
CLI->>App: invoke ps command
App->>Provider: provider.List()
Provider->>Docker: query projects & containers (labels, status, createdAt)
Docker-->>Provider: returns container/stack info
Provider-->>App: ListDeployment[] (ProviderName, TemplateID, Status, CreatedAt)
App-->>CLI: render table of deployments
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
internal/cli/ps.go (1)
24-24: Non-deterministic table row ordering due to map iteration.
c.app.Providersis amap[string]provider.Provider; ranging over it produces a different provider order on each run. Currently there's only one provider, but the design should be stable for when more are added.♻️ Suggested fix — sort provider keys before iteration
+import "sort" count := 0 -for _, provider := range c.app.Providers { +providerKeys := make([]string, 0, len(c.app.Providers)) +for k := range c.app.Providers { + providerKeys = append(providerKeys, k) +} +sort.Strings(providerKeys) +for _, key := range providerKeys { + provider := c.app.Providers[key]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/ps.go` at line 24, Iteration over c.app.Providers (a map[string]provider.Provider) produces non-deterministic row ordering; to fix, collect the map keys into a slice, sort them (e.g., sort.Strings), then iterate over the sorted keys and access providers via c.app.Providers[key] when building the table rows so output order is stable and deterministic.pkg/provider/dockercompose/utils.go (1)
177-185:toProjectNameis not injective —toTemplateIDcan't be a perfect inverse.Both
"xbow-034"and"vt-xbow-034"map to the same project name"vt-xbow-034", so the fallback can never unambiguously recover the original template ID when a template ID itself carries the"vt-"prefix. Thestrings.TrimPrefixfix above handles the common case correctly, but consider documenting the convention that template IDs must not start with"vt-"to make the round-trip deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/provider/dockercompose/utils.go` around lines 177 - 185, toProjectName is not injective: both "xbow-034" and "vt-xbow-034" map to "vt-xbow-034", so toTemplateID cannot unambiguously reverse the mapping; update the codebase to enforce or document a convention that template IDs must not start with the "vt-" prefix and add a validation check where template IDs are created/accepted (e.g., input validation path or NewTemplate function) that rejects or strips IDs starting with "vt-"; also add a clear comment on toProjectName and toTemplateID describing the invariant ("template IDs MUST NOT start with 'vt-'") so the round-trip is deterministic.internal/app/app.go (1)
14-17: RemoveStoragePathfrom Config struct and DefaultConfig().
StoragePathis unused across the codebase. Withinternal/stateandpkg/store/diskremoved in this PR, nothing references this field. Removing it from the struct definition (line 15) and its initialization inDefaultConfig()(line 35) will reduce confusion and clean up the config surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/app/app.go` around lines 14 - 17, Remove the unused StoragePath field from the Config struct and its initialization in DefaultConfig(): delete the StoragePath declaration in the Config type and remove the corresponding assignment in DefaultConfig() so the struct and default factory no longer include this unused field; update any references to Config{} construction or struct literals that set StoragePath (e.g., places calling DefaultConfig() or constructing Config values) to avoid referencing the removed field and run tests to ensure no remaining usages exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/provider/dockercompose/dockercompose.go`:
- Around line 44-51: The isRunning helper currently returns true if any
container is running, which conflicts with List()'s all-running semantics and
causes Start() to prematurely return "already running"; update isRunning to
require that every container in the provided slice has State == "running" (i.e.,
return true only when all containers are running and false otherwise) so it
matches the allRunning check used by List(), or alternatively change Start() to
perform a nuanced pre-check (using a new helper like allRunning or returning a
tri-state: none/partial/all) so Start() only short-circuits when all containers
are running; locate and update the isRunning function (and adjust Start() or
introduce allRunning if you choose the nuanced route) to ensure consistent
semantics with List().
- Around line 55-58: The code currently calls isRunning(template) and discards
its error (running, _ := d.isRunning(template)) which can hide Docker
connectivity or permission failures; change this to handle the returned error
from isRunning (check the err value), and either return that error up the call
stack or log it with context before proceeding so the pre-check cannot be
silently skipped; update the callers that expect the old behavior if needed and
ensure the logic around createDockerCLI() only runs when isRunning returned no
error.
- Around line 133-184: In the List() inner loop, don't silently ignore errors
from getProjectContainers — log the error (including stack.Name and the error)
before continuing so failures are visible; also treat stacks with zero
containers as non-deployments by skipping them instead of appending a
provider.ListDeployment with a zero CreatedAt/stack.Status (i.e., after
obtaining containers, if err != nil log and continue; then if len(containers) ==
0 continue), keeping the rest of the container-based status/createdAt logic that
sets status and CreatedAt.
In `@pkg/provider/dockercompose/utils.go`:
- Around line 187-190: toTemplateID currently returns the projectName unchanged
but must invert toProjectName by removing the "vt-" prefix; update toTemplateID
to check for the "vt-" prefix (strings.HasPrefix) and return the suffix after
"vt-" when present, otherwise return projectName unchanged so containers created
before this PR map to the original template ID; reference function toTemplateID
(and toProjectName) when making this change.
---
Nitpick comments:
In `@internal/app/app.go`:
- Around line 14-17: Remove the unused StoragePath field from the Config struct
and its initialization in DefaultConfig(): delete the StoragePath declaration in
the Config type and remove the corresponding assignment in DefaultConfig() so
the struct and default factory no longer include this unused field; update any
references to Config{} construction or struct literals that set StoragePath
(e.g., places calling DefaultConfig() or constructing Config values) to avoid
referencing the removed field and run tests to ensure no remaining usages exist.
In `@internal/cli/ps.go`:
- Line 24: Iteration over c.app.Providers (a map[string]provider.Provider)
produces non-deterministic row ordering; to fix, collect the map keys into a
slice, sort them (e.g., sort.Strings), then iterate over the sorted keys and
access providers via c.app.Providers[key] when building the table rows so output
order is stable and deterministic.
In `@pkg/provider/dockercompose/utils.go`:
- Around line 177-185: toProjectName is not injective: both "xbow-034" and
"vt-xbow-034" map to "vt-xbow-034", so toTemplateID cannot unambiguously reverse
the mapping; update the codebase to enforce or document a convention that
template IDs must not start with the "vt-" prefix and add a validation check
where template IDs are created/accepted (e.g., input validation path or
NewTemplate function) that rejects or strips IDs starting with "vt-"; also add a
clear comment on toProjectName and toTemplateID describing the invariant
("template IDs MUST NOT start with 'vt-'") so the round-trip is deterministic.
| for _, c := range containers { | ||
| if c.State == "running" { | ||
| return true, nil | ||
| } | ||
| } | ||
|
|
||
| return false, nil | ||
| } |
There was a problem hiding this comment.
isRunning uses "any running" semantics while List() requires "all running" — creates a confusing dead zone.
If a deployment is partially running (e.g., 1 of 3 containers up), isRunning returns true, causing Start() to return "already running". However, List() skips partially-running deployments (allRunning check), so vt ps shows nothing. Users see an error from start with no corresponding entry from ps and no way to recover without manually inspecting Docker.
Consider aligning isRunning with the same "all running" semantics, or having Start() perform a more nuanced pre-check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/provider/dockercompose/dockercompose.go` around lines 44 - 51, The
isRunning helper currently returns true if any container is running, which
conflicts with List()'s all-running semantics and causes Start() to prematurely
return "already running"; update isRunning to require that every container in
the provided slice has State == "running" (i.e., return true only when all
containers are running and false otherwise) so it matches the allRunning check
used by List(), or alternatively change Start() to perform a nuanced pre-check
(using a new helper like allRunning or returning a tri-state: none/partial/all)
so Start() only short-circuits when all containers are running; locate and
update the isRunning function (and adjust Start() or introduce allRunning if you
choose the nuanced route) to ensure consistent semantics with List().
| running, _ := d.isRunning(template) //nolint:errcheck | ||
| if running { | ||
| return fmt.Errorf("already running") | ||
| } |
There was a problem hiding this comment.
Silently discarding the isRunning error is risky and the //nolint:errcheck suppresses the warning.
If Docker connectivity fails specifically for the container-listing call (e.g., permission on a specific project), running is false (zero value) and execution falls through to createDockerCLI(). While createDockerCLI() would likely also fail, a transient/targeted error could allow a duplicate start to be attempted without the caller knowing the pre-check was skipped. At minimum, log the error or propagate it.
🐛 Proposed fix
- running, _ := d.isRunning(template) //nolint:errcheck
- if running {
+ running, err := d.isRunning(template)
+ if err != nil {
+ return fmt.Errorf("failed to check running state: %w", err)
+ }
+ if running {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| running, _ := d.isRunning(template) //nolint:errcheck | |
| if running { | |
| return fmt.Errorf("already running") | |
| } | |
| running, err := d.isRunning(template) | |
| if err != nil { | |
| return fmt.Errorf("failed to check running state: %w", err) | |
| } | |
| if running { | |
| return fmt.Errorf("already running") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/provider/dockercompose/dockercompose.go` around lines 55 - 58, The code
currently calls isRunning(template) and discards its error (running, _ :=
d.isRunning(template)) which can hide Docker connectivity or permission
failures; change this to handle the returned error from isRunning (check the err
value), and either return that error up the call stack or log it with context
before proceeding so the pre-check cannot be silently skipped; update the
callers that expect the old behavior if needed and ensure the logic around
createDockerCLI() only runs when isRunning returned no error.
| var deployments []provider.ListDeployment | ||
| for _, stack := range stacks { | ||
| // Get container details for template ID label and created time | ||
| containers, err := getProjectContainers(dockerCli, stack.Name) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| // Read template ID from container label (most reliable) | ||
| // Falls back to project name conversion for older containers | ||
| templateID := toTemplateID(stack.Name) | ||
| for _, c := range containers { | ||
| if id, ok := c.Labels["vt.template-id"]; ok { | ||
| templateID = id | ||
| break | ||
| } | ||
| } | ||
|
|
||
| status := stack.Status | ||
| var createdAt time.Time | ||
| if len(containers) > 0 { | ||
| // Use earliest container creation time | ||
| createdAt = time.Unix(containers[0].Created, 0) | ||
| for _, c := range containers[1:] { | ||
| t := time.Unix(c.Created, 0) | ||
| if t.Before(createdAt) { | ||
| createdAt = t | ||
| } | ||
| } | ||
|
|
||
| // Determine overall status from containers | ||
| allRunning := true | ||
| for _, c := range containers { | ||
| if c.State != "running" { | ||
| allRunning = false | ||
| break | ||
| } | ||
| } | ||
| if allRunning { | ||
| status = "running" | ||
| } else { | ||
| // Skip deployments that are not fully running | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| deployments = append(deployments, provider.ListDeployment{ | ||
| ProviderName: d.Name(), | ||
| TemplateID: templateID, | ||
| Status: status, | ||
| CreatedAt: createdAt, | ||
| }) |
There was a problem hiding this comment.
Two issues in List()'s inner loop.
1. Silent error swallow on getProjectContainers failure (lines 136–139):
When container inspection fails for a project, the error is silently discarded and the project is skipped. Add a log statement so operational problems are visible:
containers, err := getProjectContainers(dockerCli, stack.Name)
if err != nil {
+ log.Warn().Err(err).Str("project", stack.Name).Msg("failed to get containers for project")
continue
}2. Stacks with zero containers are appended with a zero CreatedAt (lines 153–176):
When len(containers) == 0, the if block is skipped entirely, so the deployment is included with time.Time{} (rendered as "0001-01-01 00:00:00") and whatever raw stack.Status Docker reports. A stack with no containers is either stopped or corrupted — skipping it is more appropriate:
🐛 Proposed fix
+ if len(containers) == 0 {
+ continue
+ }
+
status := stack.Status
var createdAt time.Time
- if len(containers) > 0 {
- // Use earliest container creation time
- ...
- }
+ // Use earliest container creation time
+ createdAt = time.Unix(containers[0].Created, 0)
+ for _, c := range containers[1:] {
+ t := time.Unix(c.Created, 0)
+ if t.Before(createdAt) {
+ createdAt = t
+ }
+ }
+ // Determine overall status from containers
+ allRunning := true
+ for _, c := range containers {
+ if c.State != "running" {
+ allRunning = false
+ break
+ }
+ }
+ if allRunning {
+ status = "running"
+ } else {
+ continue
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/provider/dockercompose/dockercompose.go` around lines 133 - 184, In the
List() inner loop, don't silently ignore errors from getProjectContainers — log
the error (including stack.Name and the error) before continuing so failures are
visible; also treat stacks with zero containers as non-deployments by skipping
them instead of appending a provider.ListDeployment with a zero
CreatedAt/stack.Status (i.e., after obtaining containers, if err != nil log and
continue; then if len(containers) == 0 continue), keeping the rest of the
container-based status/createdAt logic that sets status and CreatedAt.
| // toTemplateID extracts the template ID from a Docker Compose project name. | ||
| func toTemplateID(projectName string) string { | ||
| return projectName | ||
| } |
There was a problem hiding this comment.
toTemplateID is a no-op — the fallback template ID reconstruction is wrong.
The function is documented as inverting toProjectName, but it returns the project name unchanged. For a template whose ID is "mytemplate", toProjectName produces "vt-mytemplate", so toTemplateID("vt-mytemplate") must return "mytemplate" — not "vt-mytemplate".
This only affects the fallback path in List() (containers missing the vt.template-id label), but that path covers all containers started before this PR lands. Any user upgrading will see incorrect TemplateID values in vt ps output until containers are restarted.
🐛 Proposed fix
// toTemplateID extracts the template ID from a Docker Compose project name.
func toTemplateID(projectName string) string {
- return projectName
+ return strings.TrimPrefix(projectName, vtProjectPrefix)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // toTemplateID extracts the template ID from a Docker Compose project name. | |
| func toTemplateID(projectName string) string { | |
| return projectName | |
| } | |
| // toTemplateID extracts the template ID from a Docker Compose project name. | |
| func toTemplateID(projectName string) string { | |
| return strings.TrimPrefix(projectName, vtProjectPrefix) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/provider/dockercompose/utils.go` around lines 187 - 190, toTemplateID
currently returns the projectName unchanged but must invert toProjectName by
removing the "vt-" prefix; update toTemplateID to check for the "vt-" prefix
(strings.HasPrefix) and return the suffix after "vt-" when present, otherwise
return projectName unchanged so containers created before this PR map to the
original template ID; reference function toTemplateID (and toProjectName) when
making this change.
|
We retained BoltDB because users can create stacks with the same naming convention. Without persistent storage, the tool may mistakenly dispose of unrelated docker compose stacks. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
internal/cli/cli.go (2)
45-47:Shortis purely decorative — may look odd in non-help contexts.
Shortis used by cobra in several places beyond the custom help template (e.g., parent command listings, shell completions). A rainbow separator line will appear as garbled ANSI escape codes in contexts that don't render colors (piped output, completion scripts). Consider keeping a meaningful short description and moving the decorative separator into the custom usage template only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/cli.go` around lines 45 - 47, The cobra root command initialization sets c.rootCmd with a decorative ANSI rainbow string in the Short field; replace that with a concise, meaningful description (e.g., "vt — CLI for <purpose>") by updating the Short property on the cobra.Command, and move the decorative separator into the custom help/usage template or banner rendering code so it is only shown in colorized help output; refer to c.rootCmd and the cobra.Command literal to locate and change the Short value and adjust the help/usage template where the banner is currently injected.
90-96: Stale comment: "animated banner" → banner is now static.Line 90 still says "animated banner" but the banner was simplified to a static output in this PR.
Proposed fix
- // Custom help function that prints animated banner first + // Custom help function that prints banner first🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/cli.go` around lines 90 - 96, The inline comment above the custom help function is outdated: change "Custom help function that prints animated banner first" to reflect the banner is static now (e.g., "Custom help function that prints static banner first"); update the comment near c.rootCmd.SetHelpFunc and banner.Print to remove "animated" so it accurately describes the behavior.internal/banner/banner.go (2)
81-94: Trailing whitespace/tabs in the banner literal (Line 92).Line 92 has a long run of mixed tabs and spaces before the
AppVersioninterpolation. This will render as invisible whitespace in the terminal. If the intent is right-alignment, consider usingfmt.Sprintfwith explicit padding instead, for clarity and to avoid accidental whitespace drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/banner/banner.go` around lines 81 - 94, The banner literal contains invisible trailing mixed tabs/spaces before the AppVersion interpolation in Banner(), causing accidental whitespace; remove the literal trailing whitespace and replace the manual spacing with an explicit formatted padding call (use fmt.Sprintf with a width or right-alignment specifier) when inserting AppVersion so the version is placed at the intended column reliably; update the concatenation that currently includes fmt.Sprintf("\033[1m%s\033[0m", AppVersion) to use a single fmt.Sprintf that applies the color formatting and explicit padding, leaving the raw banner lines free of trailing tabs/spaces and relying on formatted output for alignment.
22-28: Emptyconstblock with only commented-out entries.This block compiles to nothing and adds visual noise. Either remove it entirely or gate the colors behind a build tag / unexported var if you expect to reuse them soon.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/banner/banner.go` around lines 22 - 28, Remove the empty commented const block in internal/banner/banner.go to eliminate visual noise; if you intend to reuse the color escape sequences later, instead declare them as actual unexported constants (e.g., yellow, cyan, magenta, gray, reset) or place them behind a specific build tag so they are only compiled when needed—update the const block accordingly or delete it entirely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/banner/banner.go`:
- Around line 103-106: The cast uintptr→int in isTerminal causes gosec G115; fix
by reading fd := os.Stdout.Fd(), validating it fits into int (e.g. compare fd <=
uintptr(math.MaxInt)), and only then call term.IsTerminal(int(fd)); if it
doesn't fit, return false (or handle appropriately). Update the isTerminal
function to perform the bounds check using math.MaxInt and use
term.IsTerminal(int(fd)) after the check to satisfy the linter.
---
Nitpick comments:
In `@internal/banner/banner.go`:
- Around line 81-94: The banner literal contains invisible trailing mixed
tabs/spaces before the AppVersion interpolation in Banner(), causing accidental
whitespace; remove the literal trailing whitespace and replace the manual
spacing with an explicit formatted padding call (use fmt.Sprintf with a width or
right-alignment specifier) when inserting AppVersion so the version is placed at
the intended column reliably; update the concatenation that currently includes
fmt.Sprintf("\033[1m%s\033[0m", AppVersion) to use a single fmt.Sprintf that
applies the color formatting and explicit padding, leaving the raw banner lines
free of trailing tabs/spaces and relying on formatted output for alignment.
- Around line 22-28: Remove the empty commented const block in
internal/banner/banner.go to eliminate visual noise; if you intend to reuse the
color escape sequences later, instead declare them as actual unexported
constants (e.g., yellow, cyan, magenta, gray, reset) or place them behind a
specific build tag so they are only compiled when needed—update the const block
accordingly or delete it entirely.
In `@internal/cli/cli.go`:
- Around line 45-47: The cobra root command initialization sets c.rootCmd with a
decorative ANSI rainbow string in the Short field; replace that with a concise,
meaningful description (e.g., "vt — CLI for <purpose>") by updating the Short
property on the cobra.Command, and move the decorative separator into the custom
help/usage template or banner rendering code so it is only shown in colorized
help output; refer to c.rootCmd and the cobra.Command literal to locate and
change the Short value and adjust the help/usage template where the banner is
currently injected.
- Around line 90-96: The inline comment above the custom help function is
outdated: change "Custom help function that prints animated banner first" to
reflect the banner is static now (e.g., "Custom help function that prints static
banner first"); update the comment near c.rootCmd.SetHelpFunc and banner.Print
to remove "animated" so it accurately describes the behavior.
internal/banner/banner.go
Outdated
| // isTerminal checks if stdout is a terminal | ||
| func isTerminal() bool { | ||
| return term.IsTerminal(int(os.Stdout.Fd())) | ||
| } |
There was a problem hiding this comment.
Fix G115: integer overflow conversion uintptr → int — this is failing CI.
os.Stdout.Fd() returns uintptr, and the direct cast to int triggers gosec G115. Use a bounds check or cast through a fixed-width type to satisfy the linter.
🔧 Proposed fix
-func isTerminal() bool {
- return term.IsTerminal(int(os.Stdout.Fd()))
+func isTerminal() bool {
+ fd := os.Stdout.Fd()
+ //nolint:gosec // Fd() returns a small file descriptor; overflow is not realistic.
+ return term.IsTerminal(int(fd))
}Alternatively, if the project prefers not to suppress:
+import "math"
+
func isTerminal() bool {
- return term.IsTerminal(int(os.Stdout.Fd()))
+ fd := os.Stdout.Fd()
+ if fd > math.MaxInt {
+ return false
+ }
+ return term.IsTerminal(int(fd))
}🧰 Tools
🪛 GitHub Actions: 🔨 Tests
[error] 105-105: Gosec: G115 - integer overflow conversion uintptr -> int (golangci-lint)
🪛 GitHub Check: Lint
[failure] 105-105:
G115: integer overflow conversion uintptr -> int (gosec)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/banner/banner.go` around lines 103 - 106, The cast uintptr→int in
isTerminal causes gosec G115; fix by reading fd := os.Stdout.Fd(), validating it
fits into int (e.g. compare fd <= uintptr(math.MaxInt)), and only then call
term.IsTerminal(int(fd)); if it doesn't fit, return false (or handle
appropriately). Update the isTerminal function to perform the bounds check using
math.MaxInt and use term.IsTerminal(int(fd)) after the check to satisfy the
linter.
3a0efde to
35cff2b
Compare

#155
Summary by CodeRabbit
New Features
Refactor
Docs