Skip to content

refactor: reorganize dirs#3683

Merged
aljazerzen merged 29 commits intomainfrom
reorganize-dirs
Oct 18, 2023
Merged

refactor: reorganize dirs#3683
aljazerzen merged 29 commits intomainfrom
reorganize-dirs

Conversation

@aljazerzen
Copy link
Copy Markdown
Member

@aljazerzen aljazerzen commented Oct 15, 2023

Combine compiler "things" into its own dir.

This basically says "bindings are not PRQL bindings, but prqlc bindings". Similar for packages/.

The caveat is that I've shortened the prqlc/crates/prql-parser/ to prqlc/crates/parser, but kept the crate name prql-parser. We can update this, but I don't want to change the published names "prql-compiler" and "prqlc".

@aljazerzen aljazerzen changed the title reorganize dirs refactor: reorganize dirs Oct 15, 2023
Copy link
Copy Markdown
Member

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

I'm OK with this — it's purely internal.

I'm pro-the approach described at https://matklad.github.io/2021/08/22/large-rust-workspaces.html, but given your contributions I would weigh your vote very highly here.

Comment thread Cargo.toml Outdated
Comment thread prqlc/crates/compiler/README.md
Comment on lines +15 to +17
".direnv": true,
".pytest_cache": true,
".mypy_cache": true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW I have these in my global settings rather than per-workspace...

@max-sixty
Copy link
Copy Markdown
Member

I see the upgrades have caused merge conflicts. They should be mostly done (they come in each Monday), but feel free to pause them if it's causing you trouble.

@max-sixty
Copy link
Copy Markdown
Member

@aljazerzen lmk if you want a hand merging this! I worry that as more changes come from elsewhere, git might not be smart enough and the merge will be really difficult.

I don't think there's much we can "split off" — you already merged things like the justfile separately...

@aljazerzen
Copy link
Copy Markdown
Member Author

Yeah, that'd be great, if you have the time.

@aljazerzen aljazerzen merged commit a9d81a4 into main Oct 18, 2023
@aljazerzen aljazerzen deleted the reorganize-dirs branch October 18, 2023 17:11
aljazerzen added a commit that referenced this pull request Oct 18, 2023
Followup for #3683

Because I've moved a bunch of files, links to github.com repo main branch were broken.
So I've disabled the check, merged and now they should work again.
@max-sixty
Copy link
Copy Markdown
Member

Yeah, that'd be great, if you have the time.

You merged 2 hours later! I'm too online, but not that online. :D

@aljazerzen
Copy link
Copy Markdown
Member Author

I though more things need fixing :D

@eitsupi
Copy link
Copy Markdown
Member

eitsupi commented Oct 19, 2023

Great work!

@aljazerzen
Copy link
Copy Markdown
Member Author

@eitsupi I was a bit worried you'd have objections to the reorganization, since you have done a lot of work on the bindings, so hearing praises from you means a lot!

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.

3 participants