Skip to content

[DEPRECATED] Implement Display trait for some built-in types #220

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

Closed
wants to merge 0 commits into from

Conversation

hapenia
Copy link
Contributor

@hapenia hapenia commented Apr 8, 2023

related to #110 .

@hapenia hapenia changed the title Implement 'Display' trait for some built-in types Implement Display trait for some built-in types Apr 8, 2023
@Bromeon Bromeon added feature Adds functionality to the library c: core Core components labels Apr 8, 2023
@Bromeon
Copy link
Member

Bromeon commented Apr 8, 2023

Thanks a lot!

Is this the Godot str() representation of these types?

Instead of separate tests and // comments inside the implementation, the string representation could be documented with /// and be part of a Rust doctest. That way, the user would see how it looks and we would verify it. One example per type should already be enough.

Please also squash your commits as per contribution guidelines. And if possible, avoid commit messages like containing # + a number, as GitHub interprets those as issue references and creates a lot of cross-referencing spam. Thank you 🙂

@Bromeon
Copy link
Member

Bromeon commented Apr 9, 2023

I assume the close was unintentional, since you pushed to the branch afterwards?

@hapenia hapenia marked this pull request as draft April 9, 2023 13:45
@hapenia hapenia closed this Apr 12, 2023
@hapenia hapenia changed the title Implement Display trait for some built-in types [DEPRECATED] Implement Display trait for some built-in types Apr 12, 2023
@hapenia
Copy link
Contributor Author

hapenia commented Apr 12, 2023

Deprecated. Something goes wrong when I try to solve conflict with upstream updates. I will open a new pull request and redo changes.

@Bromeon
Copy link
Member

Bromeon commented Apr 12, 2023

In the future, please force-push the relevant changes to your branch:

git push --force-with-lease
# or if you don't care about discarding any upstream work:
git push --force

Not opening 2 PRs for the same thing helps keeping an overview. Thanks! 🙂

Superseded by #224.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants