Skip to content
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

Cleanup and update names #2

Merged
merged 4 commits into from
Dec 19, 2024
Merged

Cleanup and update names #2

merged 4 commits into from
Dec 19, 2024

Conversation

ethanuppal
Copy link
Member

This PR makes the following changes:

  • Renames tru -> true and fals -> false.
  • Replaces the older mod.rs structure with the newer idiomatic structure.
  • Removes some redundant code.

Some questions: does #[inline] actually achieve anything (on the constructors for truth-y values)? Won't the compiler be smart enough to choose to inline (or not) when it deems necessary?

@ethanuppal ethanuppal self-assigned this Nov 29, 2024
@ethanuppal ethanuppal added the refactor Improves organization or layout of code label Nov 29, 2024
@ethanuppal ethanuppal requested a review from ekiwi November 29, 2024 21:51
Copy link
Collaborator

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

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

I like how you renamed the files (I did not know that you could do that this way). The small fixes are also great.

However, please remove the renaming of fals and tru. I do not want to change that.

@ethanuppal
Copy link
Member Author

I'm curious why you prefer tru over true and fals over false?

@ekiwi
Copy link
Collaborator

ekiwi commented Dec 2, 2024

I'm curious why you prefer tru over true and fals over false?

I don't see a reason to change it.

@ethanuppal
Copy link
Member Author

I'm curious why you prefer tru over true and fals over false?

I don't see a reason to change it.

It follows established style guidelines, with examples including:

@ekiwi
Copy link
Collaborator

ekiwi commented Dec 2, 2024

It does not matter. I currently do not want to do the work involved in changing all uses of this API without added functionality.

@ethanuppal
Copy link
Member Author

I deprecated the old names, so it should still work.

@ethanuppal
Copy link
Member Author

@ekiwi

@ekiwi
Copy link
Collaborator

ekiwi commented Dec 9, 2024

I like how you renamed the files (I did not know that you could do that this way). The small fixes are also great.

However, please remove the renaming of fals and tru. I do not want to change that.

@ekiwi ekiwi merged commit 0932b0e into main Dec 19, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Improves organization or layout of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants