Skip to content

Variable Rename: props to property_entries#308

Merged
elementbound merged 13 commits intofoxssake:refactor/rename-varsfrom
TheYellowArchitect:record-input-properties
Nov 3, 2024
Merged

Variable Rename: props to property_entries#308
elementbound merged 13 commits intofoxssake:refactor/rename-varsfrom
TheYellowArchitect:record-input-properties

Conversation

@TheYellowArchitect
Copy link
Copy Markdown
Contributor

@TheYellowArchitect TheYellowArchitect commented Oct 16, 2024

It is easy to be confused with the variable names because of no static typings.
properties for example are not properties but PropertyEntries.
It makes it confusing to the reader as to what any variable is, especially new users to the codebase.

This PR had to be made because in the netfox discord I saw someone asking 'how can I sync props' and someone asked 'properties or props' and the reply was actually props ala prop hunt lol

Basically the current naming convention causes a lot of confusion.

The first suggestion and the implementation of this is to convert props to property_entries
The second suggestion is to rename property of type String to property_name, and have the name property be the PropertyEntry itself. I am against this second suggestion because this repo has no static typing as coding convention, so property being PropertyEntry is just weird, while property_entry being of type PropertyEntry is intuitive.

@elementbound
Copy link
Copy Markdown
Contributor

this repo has no static typing as coding convention

False. There is static typing as convention, but not everywhere. Class members are statically typed.

property being PropertyEntry is just weird, while property_entry being of type PropertyEntry is intuitive

That is a subjective opinion being presented as fact.

@elementbound
Copy link
Copy Markdown
Contributor

Can you please merge #308, #306 and #305 into a single PR? We could merge them in a single go with a patch version bump, e.g.

chore: Rename variables

@elementbound elementbound changed the base branch from main to refactor/rename-vars November 3, 2024 09:42
@elementbound elementbound merged commit f1fc3d1 into foxssake:refactor/rename-vars Nov 3, 2024
@TheYellowArchitect TheYellowArchitect deleted the record-input-properties branch November 5, 2024 16:10
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