Skip to content

Adds explicit search command for better UX#2

Merged
scalvert merged 8 commits intomainfrom
add-search-command
Feb 1, 2025
Merged

Adds explicit search command for better UX#2
scalvert merged 8 commits intomainfrom
add-search-command

Conversation

@scalvert
Copy link
Contributor

@scalvert scalvert commented Jan 30, 2025

Summary

⚠️ This PR got WAAAAAAAY out of control in terms of size, and I probably should have split it up, but 🤷‍♂️ .

Adds a glean search ... command to provide a better UX to perform searches. This command wraps our existing http client implementation and uses that to load config, perform search, etc. Most of the code relates to providing types for marshaling/unmarshaling.

Screenshot 2025-01-30 at 7 54 24 AM

This pull request includes significant refactoring and improvements to the command structure of the CLI. The changes focus on improving code maintainability by converting existing command functions to use a more consistent and modular approach.

Refactoring and Improvements:

  • Command Structure Refactoring:

    • cmd/api.go: Renamed apiOptions to APIOptions, updated function names to follow Go naming conventions, and removed the init function in favor of returning the command from NewCmdAPI. [1] [2] [3] [4]
    • cmd/config.go: Introduced ConfigOptions struct, replaced var configCmd with NewCmdConfig function, and updated flag handling to use the new struct. [1] [2] [3] [4]
    • cmd/generate.go: Replaced var generateCmd with NewCmdGenerate function.
    • cmd/openapi_spec.go: Introduced OpenAPISpecOptions struct, replaced newOpenapiSpecCmd with NewCmdOpenAPISpec function, and updated flag handling to use the new struct. [1] [2] [3]
  • Test Updates:

    • cmd/api_test.go: Adjusted tests to use the new NewCmdAPI function and removed redundant test cases.
    • cmd/config_test.go: Updated tests to use the new NewCmdConfig function and simplified test cases.
    • cmd/generate_test.go: Adjusted tests to use the new NewCmdGenerate function.
    • cmd/openapi_spec_test.go: Updated tests to use the new NewCmdOpenAPISpec function, removed mock client setup, and simplified test case structure. [1] [2] [3]
  • Root Command Update:

    • cmd/root.go: Introduced NewCmdRoot function to create the root command, replacing the previous var rootCmd.

@scalvert scalvert requested a review from Copilot January 30, 2025 15:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

@scalvert scalvert requested a review from Copilot February 1, 2025 20:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 7 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (13)
  • go.mod: Language not supported
  • cmd/api.go: Evaluated as low risk
  • cmd/api_test.go: Evaluated as low risk
  • cmd/config.go: Evaluated as low risk
  • cmd/config_test.go: Evaluated as low risk
  • cmd/generate.go: Evaluated as low risk
  • cmd/generate_test.go: Evaluated as low risk
  • taskfile.yml: Evaluated as low risk
  • pkg/testutils/setup.go: Evaluated as low risk
  • pkg/testutils/mock_client.go: Evaluated as low risk
  • pkg/llm/openapi_test.go: Evaluated as low risk
  • cmd/root_test.go: Evaluated as low risk
  • cmd/openapi_spec.go: Evaluated as low risk
Comments suppressed due to low confidence (1)

pkg/config/config.go:22

  • Setting ConfigPath in the init function might not be ideal for testing purposes. Consider setting it in a separate function that can be called explicitly.
func init() {

@scalvert scalvert marked this pull request as ready for review February 1, 2025 20:33
@scalvert
Copy link
Contributor Author

scalvert commented Feb 1, 2025

Just going to merge this in and effectively reset the state of the repo.

@scalvert scalvert merged commit 8a39829 into main Feb 1, 2025
10 checks passed
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