Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tapdb+sqlc: sqlc: add script to merge SQL migrations into consolidated schemas #1387

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

Roasbeef
Copy link
Member

In this PR, we introduce a new Go script located in
cmd/merge-sql-schemas/main.go that consolidates SQL migration files
into a single schema file. The script reads all .up.sql migration
files from the tapdb/sqlc/migrations directory, executes them in order
on an in-memory SQLite database, and retrieves the final schema
definitions for tables and views.

Additionally, the Makefile is updated to include a call to this new
script during the SQL code generation process. The .gitignore file is
that temporary or unwanted files do not clutter the repository.
also modified to exclude files matching the pattern .aider*, ensuring

When I was making #1386, I made a few mistakes along the way as I was looking at the initial migration file, but later migration files ended up modifying a schema I was trying to migrate. With this, devs can look in a single place for the latest set of schemas.

Fixes #1062

@coveralls
Copy link

coveralls commented Feb 14, 2025

Pull Request Test Coverage Report for Build 13404212592

Details

  • 0 of 62 (0.0%) changed or added relevant lines in 1 file are covered.
  • 20 unchanged lines in 9 files lost coverage.
  • Overall coverage decreased (-0.003%) to 54.455%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/merge-sql-schemas/main.go 0 62 0.0%
Files with Coverage Reduction New Missed Lines %
commitment/tap.go 1 84.55%
address/mock.go 2 97.39%
asset/group_key.go 2 72.65%
asset/mock.go 2 72.45%
tapchannel/aux_invoice_manager.go 2 89.56%
tapgarden/custodian.go 2 76.34%
rfqmsg/records.go 3 71.2%
tapchannel/aux_leaf_signer.go 3 43.43%
universe/base.go 3 79.25%
Totals Coverage Status
Change from base Build 13397714787: -0.003%
Covered Lines: 48651
Relevant Lines: 89342

💛 - Coveralls

@guggero guggero self-requested a review February 17, 2025 08:34
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice! Very useful 🎉

@Roasbeef Roasbeef requested review from guggero, a team and GeorgeTsagk and removed request for a team February 18, 2025 01:04
Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

LGTM 🤖

This commit introduces a new Go script located in
`cmd/merge-sql-schemas/main.go` that consolidates SQL migration files
into a single schema file. The script reads all `.up.sql` migration
files from the `tapdb/sqlc/migrations` directory, executes them in order
on an in-memory SQLite database, and retrieves the final schema
definitions for tables and views.

Additionally, the Makefile is updated to include a call to this new
script during the SQL code generation process. The `.gitignore` file is
that temporary or unwanted files do not clutter the repository.
also modified to exclude files matching the pattern `.aider*`, ensuring
This commit checks in the unified schema using the script added in the
prior commit.
@@ -252,9 +252,17 @@ sqlc:
@$(call print, "Merging SQL migrations into consolidated schemas")
go run ./cmd/merge-sql-schemas/main.go

sqlc-check: sqlc
sqlc-check:
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the dependency on the sqlc target? Now it won't actually generate the schema anymore to see if there's a diff.

Copy link
Member Author

Choose a reason for hiding this comment

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

My bad, missed this. I removed it to verify that the bash fragment I had worked (so I could remove the file, then run the check w/o it adding the file again).

@Roasbeef Roasbeef merged commit e7f4f5b into lightninglabs:main Feb 20, 2025
18 checks passed
guggero added a commit that referenced this pull request Feb 20, 2025
Fixes incorrect removal of the sqlc target in #1387.
Without this dependency, the check doesn't actually check anything.
@guggero guggero mentioned this pull request Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[feature]: Consolidate SQL Schema into a Single SQL File and Integrate with CI
4 participants