Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for .pgschemaignore files, allowing users to exclude specific database objects from pgschema operations. It addresses issue #24 by adding a comprehensive ignore system that supports wildcard patterns and negation rules for selective schema management.
- Adds a new
internal/ignorepackage with TOML-based configuration support - Integrates ignore functionality across dump, plan, and apply commands
- Updates IR parser and inspector to respect ignore patterns during schema analysis
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ignore/ignore.go | Core ignore functionality with pattern matching logic |
| internal/ignore/loader.go | TOML configuration file loading and parsing |
| internal/ir/parser.go | Updated to accept and use ignore configuration |
| internal/ir/inspector.go | Database inspection now respects ignore patterns |
| cmd/plan/plan.go | Loads ignore config for plan generation |
| cmd/dump/dump.go | Integrates ignore functionality into dump command |
| go.mod | Adds BurntSushi/toml dependency for TOML parsing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -38,7 +38,7 @@ func discoverTestDataVersions(testdataDir string) ([]string, error) { | |||
|
|
|||
| // parseSQL is a helper function to convert SQL string to IR for tests | |||
| func parseSQL(t *testing.T, sql string) *ir.IR { | |||
There was a problem hiding this comment.
[nitpick] The NewParser function now requires two parameters (defaultSchema and ignoreConfig), but this call passes nil for ignoreConfig. Consider using a more explicit parameter name or adding a comment to clarify that nil means no ignore configuration will be applied.
| func parseSQL(t *testing.T, sql string) *ir.IR { | |
| func parseSQL(t *testing.T, sql string) *ir.IR { | |
| // Pass nil for ignoreConfig to indicate no ignore configuration will be applied. |
| func NewParser(defaultSchema string, ignoreConfig *ignore.IgnoreConfig) *Parser { | ||
| if defaultSchema == "" { | ||
| defaultSchema = "public" | ||
| } | ||
| return &Parser{ | ||
| schema: NewIR(), | ||
| defaultSchema: defaultSchema, | ||
| ignoreConfig: ignoreConfig, | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a breaking API change. The NewParser function signature has changed from no parameters to requiring two parameters. All existing callers need to be updated to provide both defaultSchema and ignoreConfig parameters.
| func NewInspector(db *sql.DB, ignoreConfig *ignore.IgnoreConfig) *Inspector { | ||
| return &Inspector{ | ||
| db: db, | ||
| queries: queries.New(db), | ||
| db: db, | ||
| queries: queries.New(db), | ||
| ignoreConfig: ignoreConfig, | ||
| } | ||
| } |
There was a problem hiding this comment.
This is also a breaking API change. The NewInspector function signature has changed from one parameter to two parameters. All existing callers must be updated to provide the ignoreConfig parameter.
1 participant
Fixing #24