Skip to content

Refactor the analytics command#321

Merged
pvdz merged 1 commit intomainfrom
ref_analytics
Feb 13, 2025
Merged

Refactor the analytics command#321
pvdz merged 1 commit intomainfrom
ref_analytics

Conversation

@pvdz
Copy link
Contributor

@pvdz pvdz commented Feb 13, 2025

This refactors the analytics command:

  • Moves it into its own folder
  • Separates the CLI side of things from the actual command
  • Not having index.ts makes debugging and development easier (no more "guess the index.ts" roulette)
    • I'm torn between file naming. run-foo makes sense because it's a verb whereas foo-command makes sense because it's a noun. So now neither the type nor the command will be consistently grouped together... Perhaps it should be cmd-foo instead and the actual functions a name that matches the main function?
  • Cleans up imports that were deep-importing rather than from the top
  • Eliminates a bunch of any's
  • Functions explicitly typed
  • Flags explicitly typed
  • The command file has its parameters and details clearly at the top
  • The command file deals with CLI flag handling and what not
  • The run file deals with running the actual thing
  • Did not change semantics of the actual thing (or cli), may want to look into that later
  • The CliCommandConfig type is a bit of a trial. May refine that as more commands are refactored this way.
  • (Probably true for the structure on a whole)

@pvdz pvdz requested a review from jdalton February 13, 2025 14:37
@pvdz pvdz merged commit c6a6695 into main Feb 13, 2025
15 checks passed
@pvdz pvdz deleted the ref_analytics branch February 13, 2025 15:07
@pvdz pvdz mentioned this pull request Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants