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

Improve cmd/schemagen #262

Merged
merged 2 commits into from
Mar 17, 2024
Merged

Improve cmd/schemagen #262

merged 2 commits into from
Mar 17, 2024

Conversation

iwittkau
Copy link
Contributor

@iwittkau iwittkau commented Dec 20, 2023

We want to be able to ignore certain tables (e.g. gocqlx_migrate), views and indexes when using cmd/schemagen.

To do this, we have implemented two new flags:

  • ignore, a comma-separated list of table, view or index names to ignore, this also automatically removes any unused types
  • ignore-indexes, a bool flag that controls whether types for indexes should be generated

We have verified the implementation by extending the existing tests.

@roydahan
Copy link
Collaborator

@sylwiaszunejko can you please review this one?

@roydahan roydahan requested a review from zimnx December 24, 2023 23:03
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@iwittkau iwittkau left a comment

Choose a reason for hiding this comment

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

Thanks for the review, much appreciated!

cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
@iwittkau
Copy link
Contributor Author

iwittkau commented Jan 4, 2024

@sylwiaszunejko I added fixup commits, let me know when I can rebase the branch.

@sylwiaszunejko
Copy link
Collaborator

@sylwiaszunejko I added fixup commits, let me know when I can rebase the branch.

It looks good now, squash the clean up commits into the previous ones to make the history of changes cleaner and for me it LGTM

@mykaul
Copy link

mykaul commented Jan 8, 2024

@zimnx (or @avelanarius ) - can either of you review this?

@iwittkau
Copy link
Contributor Author

Hey @zimnx can you take a look, please? 🙏🏻

@avelanarius
Copy link

I'll review it

cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
cmd/schemagen/schemagen.go Outdated Show resolved Hide resolved
@zimnx
Copy link
Collaborator

zimnx commented Jan 23, 2024

Please squash your commits into set of independent changes. Cleanup commit affecting files changed in other PR commit should be squashed.

Co-authored by Alexander Setzer <Alexander.Setzer@alfatraining.de>
Co-authored by Alexander Setzer <Alexander.Setzer@alfatraining.de>
Copy link
Collaborator

@zimnx zimnx left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for updates!

@iwittkau
Copy link
Contributor Author

Who can press the button to merge this?

@zimnx
Copy link
Collaborator

zimnx commented Feb 26, 2024

Who can press the button to merge this?

That would be @avelanarius

@iwittkau
Copy link
Contributor Author

@avelanarius can you merge this, please?

@roydahan roydahan merged commit 8bda349 into scylladb:master Mar 17, 2024
1 check passed
@annismckenzie annismckenzie deleted the improve-schemagen branch April 3, 2024 15:02
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.

6 participants