Skip to content

Comments

fix: PGHOST, PGPORT dotenv#41

Merged
tianzhou merged 1 commit intomainfrom
env
Sep 25, 2025
Merged

fix: PGHOST, PGPORT dotenv#41
tianzhou merged 1 commit intomainfrom
env

Conversation

@tianzhou
Copy link
Contributor

Fix #40

Copilot AI review requested due to automatic review settings September 25, 2025 03:38
Copy link
Contributor

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.

Pull Request Overview

This PR fixes environment variable handling for PGHOST and PGPORT by consolidating dotenv functionality into PreRun functions rather than flag defaults.

  • Refactored environment variable handling to use PreRun functions instead of flag defaults
  • Created new PreRun function variants to handle optional connection parameters (host, port, app name)
  • Added comprehensive tests for the environment variable utility functions

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cmd/util/env.go Added new PreRun function variants to handle host, port, and app name environment variables
cmd/util/env_test.go Added comprehensive test coverage for environment variable utility functions
cmd/dump/dump.go Updated to use new PreRun function and removed env defaults from flag definitions
cmd/plan/plan.go Updated to use new PreRun function and removed env defaults from flag definitions
cmd/apply/apply.go Updated to use new PreRun function with app name support and removed env defaults from flag definitions
cmd/dump/dump_test.go Removed duplicate test functions that are now covered in the util package

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// PreRunEWithEnvVars creates a PreRunE function that validates required database connection parameters
// It checks environment variables if the corresponding flags weren't explicitly set
func PreRunEWithEnvVars(dbPtr, userPtr *string) func(*cobra.Command, []string) error {
return PreRunEWithEnvVarsAndConnection(dbPtr, userPtr, nil, nil)
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a more explicit approach by creating a struct to hold the optional parameters instead of passing multiple nil values. This would make the function calls clearer and reduce the risk of passing parameters in the wrong order.

Copilot uses AI. Check for mistakes.
@tianzhou tianzhou merged commit 3f75e34 into main Sep 25, 2025
2 checks passed
@tianzhou tianzhou deleted the env branch October 23, 2025 09:15
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.

Connection defaults to localhost despite .env configuration

1 participant