ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.13k stars 5.74k forks source link

Mark embedding options as advanced. #15081

Closed aarlt closed 4 months ago

ekpyron commented 4 months ago

(And even if we want do deal with this in a completely different manner, we can still do this in a new PR, i.e. this PR is better than develop alone, so we merge it and anything beyond that we can do afterwards)

cameel commented 4 months ago

Here's the relevant bit from that previous discussion:

I mean, rather than docs just better names would be nice. For example. STRICT_JSONCPP_VERSION sounds like it works like STRICT_Z3_VERSION. But instead it disables the version check completely. Why not something like ANY_JSONCPP_VERSION or ENABLE_JSONCPP_VERSION_CHECK?

Or USE_SYSTEM_LIBRARIES is very broad and I'd expect it to cover all libs like Boost, Z3 etc. This name needs some qualifier. The description of the option does not clarify it either.

Overall, this does not need to be extensively documented since I assume it's for development only, but at least names should be less ambiguous so that if you guess what it does, you have a chance to guess right :)

So basically, these options should just have a better names and descriptions.

The whole discussion on Matrix was longer, and there were other things mentioned, including whether we need this feature in the first place, but ultimately if we do have it, that's the one thing that should be improved IMO.

cameel commented 4 months ago

Also, a link to the original PR: #14860. I wanted to find it and expected that I would be somewhere in here already but apparently not.