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

Add feature: new property snowflake to Hashable class. #901

Closed
wants to merge 4 commits into from
Closed

Add feature: new property snowflake to Hashable class. #901

wants to merge 4 commits into from

Conversation

yatochka-dev
Copy link

@yatochka-dev yatochka-dev commented Jan 2, 2023

Summary

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added t: enhancement New feature s: needs review Issue/PR is awaiting reviews labels Jan 2, 2023
@shiftinv
Copy link
Member

shiftinv commented Jan 4, 2023

Not entirely sold on this addition yet; what issue does it solve, as opposed to using f"{o.id}"/str(o.id) directly? In terms of performance, it's about the same as str(o.id), as both are function calls in the end.

The naming is slightly confusing, "snowflake" doesn't necessarily have to mean "string" in general.

@yatochka-dev yatochka-dev closed this by deleting the head repository Jan 8, 2023
@onerandomusername onerandomusername removed the s: needs review Issue/PR is awaiting reviews label Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: enhancement New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants