Skip to content

Comments

Rename "Properties" to "PropertySet"#1667

Open
PoignardAzur wants to merge 4 commits intolinebender:mainfrom
PoignardAzur:property_refactor
Open

Rename "Properties" to "PropertySet"#1667
PoignardAzur wants to merge 4 commits intolinebender:mainfrom
PoignardAzur:property_refactor

Conversation

@PoignardAzur
Copy link
Contributor

@PoignardAzur PoignardAzur commented Feb 18, 2026

Split properties.rs into (roughly) one file per struct/trait.

This is a moving-and-renaming-only PR, with no behavior changes.

Split `properties.rs` into (roughly) one file per struct/trait.
@waywardmonkeys
Copy link
Contributor

Given that this churns every open / pending change ... I'm not sure what the overall value / gain here is? Especially if we're talking about further substantial changes to properties... Help me understand?

@PoignardAzur
Copy link
Contributor Author

The short version is that I have other changes in the pipeline that define other types like "PropertyStack" or "PropertyArena" and that a "Properties" type is ambiguous compared to them.

@waywardmonkeys
Copy link
Contributor

The short version is that I have other changes in the pipeline that define other types like "PropertyStack" or "PropertyArena" and that a "Properties" type is ambiguous compared to them.

But feels a bit pre-mature because those changes wouldn't happen given some of the other stuff we want to talk about?

@PoignardAzur
Copy link
Contributor Author

Right, I was skipping some context there. We're still discussing when and how we'll integrate features from understory_properties and your experimental branch using them.

I'd still like to merge this now. "Properties" is a bad name, and I'll still want to change it even (especially?) if we depend on understory_properties. I don't think this PR will make your work harder to integrate.

Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

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

Yes, this will create churn all over the place. However, the changes themselves aren't bad. So let's just merge this sooner rather than later and move on and spend time discussing more substantive things.

@PoignardAzur PoignardAzur added this pull request to the merge queue Feb 20, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 20, 2026
Split `properties.rs` into (roughly) one file per struct/trait.

This is a moving-and-renaming-only PR, with no behavior changes.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 20, 2026
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