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

Refactor backends #8446

Closed
wants to merge 11 commits into from
Closed

Refactor backends #8446

wants to merge 11 commits into from

Conversation

tygyh
Copy link
Contributor

@tygyh tygyh commented Feb 27, 2025

This pull request focuses on modernizing the codebase by replacing C-style includes and initializations with their C++ counterparts and improving code safety and readability.

@tygyh tygyh force-pushed the Refactor-backends branch from 6ed8265 to 3bab9be Compare February 27, 2025 20:23
@tygyh tygyh force-pushed the Refactor-backends branch from 3bab9be to e896d0b Compare February 27, 2025 20:33
@PathogenDavid
Copy link
Contributor

It's usually best to open issues to discuss things before undergoing significant work on any open source project, especially when you aren't already involved in its community.

It's also poor form to submit style changes to any codebase without prior discussion. Basically everything you've changed is a matter of taste. It is not an accident that Dear ImGui is a C+ subset of C++.

It feels like you blindly followed a bunch of linter recommendations without consideration for whether the recommendation is valid. (For example some of the parens you removed were definitely not super necessary, but you also removed some valuable intent-clarifying groupings.)

ocornut added a commit that referenced this pull request Feb 27, 2025
@ocornut
Copy link
Owner

ocornut commented Feb 27, 2025

As David says. This seems like mostly the result of some automated processing, with little thought given.

I have in good faith reviewed every suggested change and picked very few, which I have pushed as 1aab00d.
Every other changes seemed undesirable, at best unnecessary, at worst problematic.

@ocornut ocornut closed this Feb 27, 2025
@tygyh tygyh deleted the Refactor-backends branch March 2, 2025 09:28
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.

None yet

3 participants