Skip to content

Normalize imports according to usual coding style#1040

Merged
PoignardAzur merged 6 commits intomainfrom
rearrange_imports
Jun 7, 2025
Merged

Normalize imports according to usual coding style#1040
PoignardAzur merged 6 commits intomainfrom
rearrange_imports

Conversation

@PoignardAzur
Copy link
Contributor

Replace crate::vello/kurbo/parley with vello/kurbo/parley.
Group imports with the usual std-imports-crate grouping.
Import debug_panic manually in each file instead of using syntactic macro lookup.

These changes will make the crate split easier.

@waywardmonkeys
Copy link
Contributor

I want I understand this one better. We spend a lot of effort doing the opposite and now you are undoing it with no real explanation…

@PoignardAzur
Copy link
Contributor Author

We spend a lot of effort doing the opposite

How so?

Either way, I don't see a strong reason for the use crate::vello/kurbo/parley pattern, and it's applied very inconsistently throughout the project. If it's supposed to be the desired coding style, then it's not documented anywhere.

@PoignardAzur
Copy link
Contributor Author

To be clear, I'm not removing any re-rexports, just changing internal import patterns.

@waywardmonkeys
Copy link
Contributor

I am on my phone not the computer. Maybe do a got blame in some of the lines invoking vello and Kurbo.

@PoignardAzur
Copy link
Contributor Author

PoignardAzur commented Jun 7, 2025

After doing some git blames, I see:

  • Reorganize Masonry modules #848: Replaced a lot of uses of crate::{Point, Size, etc} with crate::kurbo::{Point, Size, etc}.
  • masonry: Use Kurbo via Vello. #223: Replaced kurbo::{Point, Size, etc} with either crate::kurbo::{Point, Size, etc} or vello::kurbo::{Point, Size, etc} depending on files. The rationale was that we wanted to stop importing kurbo directly, which doesn't really apply here.

I think neither are "incompatible" with this PR, or give a direction that this PR goes against.

Overall, I think we just haven't officially decided on a standard between crate::kurbo::{xxx} and vello::kurbo::{xxx}, and we've used both throughout the codebase. I strongly prefer the latter.

@waywardmonkeys
Copy link
Contributor

I guess I don’t actually care enough to argue about it. It is all bad anyway.

@PoignardAzur
Copy link
Contributor Author

We can always agree on an import format after the crate split.

@PoignardAzur PoignardAzur added this pull request to the merge queue Jun 7, 2025
Merged via the queue into main with commit 4bd298c Jun 7, 2025
18 checks passed
@PoignardAzur PoignardAzur deleted the rearrange_imports branch June 7, 2025 15:13
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.

2 participants