Skip to content

Conversation

@ab9rf
Copy link
Member

@ab9rf ab9rf commented May 3, 2025

lots of #ifdef

i tried to keep as much commonality as possible. it gets a bit difficult in places

@myk002 myk002 added this to 51.11-r2 May 3, 2025
@github-project-automation github-project-automation bot moved this to Todo in 51.11-r2 May 3, 2025
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

ugh, that's grotty, but arguably better than having diverging implementations in different files

Comment on lines 25 to 29
#ifndef WIN32
#ifndef _DARWIN
#endif /* ! WIN32 */
#include <cstdlib>
#ifndef WIN32
Copy link
Member

Choose a reason for hiding this comment

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

maybe adding spaces would make the nesting easier to read here.

Copy link
Member Author

@ab9rf ab9rf May 3, 2025

Choose a reason for hiding this comment

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

oh that's just a mess, lemme make it better

it's because the darwin header does not include <cstdlib> but both linux and windows do. it's probably appropriate to include it in darwin as well, but honestly i have no idea anymore with darwin

@myk002 myk002 moved this from Todo to Review In Progress in 51.11-r2 May 3, 2025
@ab9rf
Copy link
Member Author

ab9rf commented May 3, 2025

ugh, that's grotty, but arguably better than having diverging implementations in different files

yeah that's where i am too. i had aligned the separate files as best i could (using side by side edit) a year or so ago, and even with that they had managed to drift a bit in that relatively limited time

@myk002 myk002 merged commit 8e6c2ed into DFHack:develop May 3, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from Review In Progress to Done in 51.11-r2 May 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants