Hi there,
A few days back one guy opened PR for my old directx bindings, asking to merge patch for 68 files where he adds version(Windows) to these files, claiming that he has some problem using the package on non-Windows platforms (well, that's expected).

While I don't consider this as a harmful I'm still somewhat skeptic about such changes that adds little to no value in my opinion.
So instead I advised him to try using dub configurations for that task, and this where he start acting weird. Basically he just tell he don't care about any advises and then just quit...

Well, ok then. But I still wanted to know what people think about such situations and hear advises on how to handle them. Should I just reject such stuff in the future, or "take down my proud" and merge? (though it's not about my proud tbh)

One can read this little conversation here https://github.com/evilrat666/directx-d/pull/10

On Saturday, 9 May 2020 at 11:55:36 UTC, evilrat wrote:
> But I still wanted to know what people think about such situations and hear advises on how to handle them. Should I just reject such stuff in the future, or "take down my proud" and merge? (though it's not about my proud tbh)
>
> One can read this little conversation here https://github.com/evilrat666/directx-d/pull/10

If you think it should not be merged, then don't merge it. Judge the pull request on the value it brings and whether it fits the project, not to please people.

Besides, you gave a good reason and a good alternative. I don't see any problem here.

Finally, he closed the PR. What is there left to talk about?

On Saturday, 9 May 2020 at 11:55:36 UTC, evilrat wrote:
> Hi there,
> A few days back one guy opened PR for my old directx bindings, asking to merge patch for 68 files where he adds version(Windows) to these files, claiming that he has some problem using the package on non-Windows platforms (well, that's expected).
>
> While I don't consider this as a harmful I'm still somewhat skeptic about such changes that adds little to no value in my opinion.
> So instead I advised him to try using dub configurations for that task, and this where he start acting weird. Basically he just tell he don't care about any advises and then just quit...
>
> Well, ok then. But I still wanted to know what people think about such situations and hear advises on how to handle them. Should I just reject such stuff in the future, or "take down my proud" and merge? (though it's not about my proud tbh)
>
> One can read this little conversation here https://github.com/evilrat666/directx-d/pull/10

You're responses seemed reasonable to me. That contributor didn't seem like he wanted to consider your point of view even though you were trying to understand his use case.

As for this particular issue, I wasn't sure why he needed those versions. And you pointed out that someone may need those bindings even if version is not Windows. The caller can always wrap their imports in a version(Windows) block but they wouldn't be able to remove that condition if it was added to the modules internally. So you'd have to enable version Windows to use them which would not work for certain use cases.

On Saturday, 9 May 2020 at 11:55:36 UTC, evilrat wrote:
> Hi there,
> A few days back one guy opened PR for my old directx bindings, asking to merge patch for 68 files where he adds version(Windows) to these files, claiming that he has some problem using the package on non-Windows platforms (well, that's expected).
>
> While I don't consider this as a harmful I'm still somewhat skeptic about such changes that adds little to no value in my opinion.
> So instead I advised him to try using dub configurations for that task, and this where he start acting weird. Basically he just tell he don't care about any advises and then just quit...
>
> Well, ok then. But I still wanted to know what people think about such situations and hear advises on how to handle them. Should I just reject such stuff in the future, or "take down my proud" and merge? (though it's not about my proud tbh)
>
> One can read this little conversation here https://github.com/evilrat666/directx-d/pull/10

Hello,

To be honest would have I preferred the version(Windows) solution to DUB configurations.

Actually DUB configurations are for software configurations (what features are in), not what _platforms_ are in. Some people use DUB configurations for examples which is really puzzling, you can just use a separate DUB description file for that.

One part of our codebase had the exact problem your contributor stumbled upon (dplug:x11) beacuse it brings a link time dependencies and, like your contributor, DUB doesn't have platform-specific dependencies.

In products we have 10 DUB configurations and cannot multiply that number by 2 for platform considerations.

So we went with version(linux) there weren't other choices.


Simply said, people often err on the side of the "too-cautious", but the more your merge, the more likely you end up with multiple strong contributors (with a ramp-up as each side soften to each other).

There is precedence for this also:

It is now implemented like this in druntime for bindings[0][1].

[0] https://github.com/dlang/druntime/blob/master/src/core/sys/windows/aclapi.d#L11
[1] https://github.com/dlang/druntime/blob/master/src/core/sys/dragonflybsd/err.d#L11

On Saturday, 9 May 2020 at 11:55:36 UTC, evilrat wrote:

> Well, ok then. But I still wanted to know what people think about such situations and hear advises on how to handle them. Should I just reject such stuff in the future, or "take down my proud" and merge? (though it's not about my proud tbh)

This is why you released your code under the MIT license and posted it on Github, where there's a "Fork" button at the top of the project page. They've already written a PR that does what they want. You're under no obligation to do anything beyond responding to the PR - you're not blocking them in any way.

On Saturday, 9 May 2020 at 11:55:36 UTC, evilrat wrote:
> Hi there,
> A few days back one guy opened PR for my old directx bindings, asking to merge patch for 68 files where he adds version(Windows) to these files, claiming that he has some problem using the package on non-Windows platforms (well, that's expected).
>
> While I don't consider this as a harmful I'm still somewhat skeptic about such changes that adds little to no value in my opinion.
> So instead I advised him to try using dub configurations for that task, and this where he start acting weird. Basically he just tell he don't care about any advises and then just quit...
>
> Well, ok then. But I still wanted to know what people think about such situations and hear advises on how to handle them. Should I just reject such stuff in the future, or "take down my proud" and merge? (though it's not about my proud tbh)
>
> One can read this little conversation here https://github.com/evilrat666/directx-d/pull/10

I partially agree with the PR.

Packages should work out of the box without additional project configuration for platform etc.

Like for vibe.d you don't have to specify any platform specific things in your configuration etc.

Platform dependencies should be handled in code and not in configuration files.

What if a new package manager came along and it made the assumption that things should work out of the box regardless of platform?

What if someone wanted to use your package without dub and just through dmd? There is no way to tell dmd the specification in an easy way.

Of course it's your package and up to you but I see it as a red flag when packages require extra configuration to be used on specific platforms.

Thank you guys. This should be enough.



On Saturday, 9 May 2020 at 12:33:43 UTC, Sebastiaan Koppe wrote:
>
> If you think it should not be merged, then don't merge it. Judge the pull request on the value it brings and whether it fits the project, not to please people.

Yes that's what I think.

> Finally, he closed the PR. What is there left to talk about?

This situation was just ridiculuos. He started nice, but then just throwed a tantrum and then seemingly rage quitted.

Normally I would just yeet him to lead developer and if he yeet him back to me I'd just yeet down such PR and be done with it.
Adding to previous paragraph this is why I was kind of thinking "WTF just happened? Did he expects I merge this because he is trying to make me feel guilty? Well I does not really care that much".

But I still bother a bit about if this guy is blaming me for his project architecture screw up or is it just me became too ignorant.
As a programmer I tend to reason a lot about of things while trying to stay above such silly egoistic motives.




On Saturday, 9 May 2020 at 12:41:39 UTC, Jonathan Marler wrote:
>
> As for this particular issue, I wasn't sure why he needed those versions. And you pointed out that someone may need those bindings even if version is not Windows. The caller can always wrap their imports in a version(Windows) block but they wouldn't be able to remove that condition if it was added to the modules internally.

Yeah that's what I thought. Another reason is that DirectX used on other platforms too, including XBOX and mobile phones. None of these can be qualified as "Windows".




On Saturday, 9 May 2020 at 13:09:41 UTC, Guillaume Piolat wrote:
>
> To be honest would have I preferred the version(Windows) solution to DUB configurations.
>
> Actually DUB configurations are for software configurations (what features are in), not what _platforms_ are in. Some people use DUB configurations for examples which is really puzzling, you can just use a separate DUB description file for that.
>
> In products we have 10 DUB configurations and cannot multiply that number by 2 for platform considerations.
>
> So we went with version(linux) there weren't other choices.

I am familiar with that problem, dealing with cartesian product of number of configurations is tough.

This looks as a good landmark for architecture seam for me.
So I do think this counts as a software configuration, after all, one can think that this rendering backend is optional and the app will be completely fine without it.

But can't this be handled a bit more elegant by using subpackages with configuration specific to that thing?

I mean he was likely doing some graphics app or basic game, so this definitely asks for rendering backend separation, and even if there is already few configurations in the main package, doing it that way should isolate the problem and prevent explosion of configurations.


> Simply said, people often err on the side of the "too-cautious", but the more your merge, the more likely you end up with multiple strong contributors (with a ramp-up as each side soften to each other).

I'm actually feeling ok with accepting contributions, I've just seen such situations before in other projects, but back then I was able to afford being ignorant thinking "Wow, he's nuts".
Now I have to rethink how to deal with this in the future.




On Sunday, 10 May 2020 at 10:40:05 UTC, bauss wrote:
>
> I partially agree with the PR.
>
> Packages should work out of the box without additional project configuration for platform etc.
>
> What if someone wanted to use your package without dub and just through dmd? There is no way to tell dmd the specification in an easy way.
>
> Of course it's your package and up to you but I see it as a red flag when packages require extra configuration to be used on specific platforms.

I am not really that familiar with how D community handles this specific problem.
However entirely disabling the whole API looks like a fail to me.
I'm not so ignorant to turn down such concerns, but at this moment I don't see a better solution for this problem, that doesn't require ANY configuration.

Like I said, this isn't just for Windows, but also should work with Windows Phone, XBOX and UWP.
None of these are covered with compiler version identifiers.
https://dlang.org/spec/version.html#predefined-versions

Anyway, I'll think what can be done to fix the problem without limiting the whole thing just to version(Windows).