• mw (27/27) Jun 23 2021 https://github.com/apache/thrift/pull/2404
• mw (3/14) Jun 23 2021 Thank jmh530 for the PR comment, but I think they want to hear
• Vladimir Panteleev (27/28) Jun 24 2021 I think the pull request needs work, though the reviewer didn't

mw <mingwu gmail.com> writes:

https://github.com/apache/thrift/pull/2404

their comments:

```
Downgrading openssl version? Because someone says something about 
some compiler? Really?
```


my reply:

```
openssl is not downgraded!

If you look closely, the openssl version in the dub.json is D's 
(not C's) openssl version, see here:
https://code.dlang.org/packages/openssl

Versions:				
2.0.3+1.1.0h | 2021-May-13
2.0.2+1.1.0h | 2021-Mar-09
2.0.1+1.1.0h | 2019-May-12
2.0.0+1.1.0h | 2018-Sep-17
1.1.6+1.0.1g | 2017-Nov-05

(the D openssl's version format is: <D openssl version>+<C 
openssl version>

For C's openssl both use_openssl_1_1 () and use_openssl_1_0 are 
supported.

Regard the popularity LDC compiler, please see the D's forum:

https://forum.dlang.org/

```

Can people here convince them LDC is an important D compiler.

Thanks.

mw <mingwu gmail.com> writes:

On Wednesday, 23 June 2021 at 20:12:46 UTC, mw wrote:
 https://github.com/apache/thrift/pull/2404

 their comments:

 ```
 Downgrading openssl version? Because someone says something 
 about some compiler? Really?
 ```


Thank jmh530 for the PR comment, but I think they want to hear 
more about LDC:


 ```
 Regard the popularity LDC compiler, please see the D's forum:

 https://forum.dlang.org/

 ```

 Can people here convince them LDC is an important D compiler.

Vladimir Panteleev <thecybershadow.lists gmail.com> writes:

On Wednesday, 23 June 2021 at 20:12:46 UTC, mw wrote:
 https://github.com/apache/thrift/pull/2404

I think the pull request needs work, though the reviewer didn't 
do a good job of explaining why.

1. It's not clear what causes the error message exactly. How 
could `llvm_bswap` conflict with `nativeToBigEndian`? They have 
different names. There should be an explanation for how the 
conflict arose, why it happens only with LDC, and how your 
proposed change fixes it.

2. It's not explained what the conflict problem has anything to 
do with OpenSSL. Your pull request description makes no mention 
about OpenSSL, yet most of the changes in the diff are concerning 
it one way or the other.

3. It's not explained what the OpenSSL changes are even needed 
for. "Fixing LDC" is not a sufficient explanation, they need to 
explain what exactly is broken, why it is broken, what is being 
changed, and how the change fixes the problem.

4. The OpenSSL changes really should be in a completely separate 
pull request.

5. If for whatever reason the OpenSSL changes cannot be in a 
separate pull request, then it should be explained why.

6. There should also be a justification for why the problem is 
being fixed by making a change (or changes) in Apache Thrift, and 
not in LDC on the D OpenSSL bindings etc.

All answers to these questions should be included in commit 
messages or the pull request description.

I suggest closing that PR, and opening a new PR (or PRs) which 
address the above.