-
Notifications
You must be signed in to change notification settings - Fork 5
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
Main Extension Distribution Pipeline #14
Conversation
@carlopi could you please take a look at the changes I've made here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, makes a lot of sense.
I think this was originally made to allow this by @samansmink, pinging him since he might be curious.
PR is good to go!
Is there a specific reason you want to build sqlsmith against both main and v1.0.0? I know @Maxxen does it too for spatial ,but I think it might be a bit much to run both always |
Thanks, @carlopi ! I'm not sure I have a good answer to @samansmink 's question. I guess we want to build sqlsmith against both main and v1.0.0 to see, if it's compatible with current main version, not only with the last stable. But should we? @Tmonster , should we build sqlsmith against both (main and v1.0.0)? How to decide? |
@hmeriann there's nothing fundamentally wrong, but it might be wasteful. Note that DuckDB CI also builds the sqlsmith extension on every PR at the commit specified here https://github.com/duckdb/duckdb/blob/18c59ed4b42d1ef4692973f82361d6bce2af7092/.github/config/out_of_tree_extensions.cmake#L104 In general I would say the most natural process for extensions is to:
However, for sqlsmith this might be different as this extension is mostly used in CI anyway so perhaps targeting main always makes sense here? Note that targeting main can be a little unstable as that means that the sqlsmith ci can suddenly break due to changes in duckdb/duckdb. |
Thanks @samansmink for the clarifications! Building sqlsmith only against current main makes more sense then |
Closing this since it's already been made in this PR #11 |
This PR removes excluding architectures as discussed here and put repeating job into a matrix