optional: use #if instead of #ifndef, fixes #3859

[theodelrieu]

Hi, it's been a while.

Just connected to Github and saw the optional-related issue, tests pass with -DJSON_ImplicitConversions=OFF.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @theodelrieu
Please read and follow the Contribution Guidelines.

[nlohmann]

Welcome back! You need Astyle 3.1 to amalgamate.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @theodelrieu
Please read and follow the Contribution Guidelines.

[theodelrieu]

Making 3.1 work on macOS is too much of a pain, I've edited the amalgamated file by hand 🤡

[nlohmann]

I downloaded it and built it myself. It's indeed a pain I hope to fix soon.

[theodelrieu]

I downloaded it and built it myself.

So did I, but running it didn't do anything, which is why I tried to edit by hand, which doesn't seem to work either...

[coveralls]

coverage: 99.186%. remained the same
when pulling 39807e2 on theodelrieu:develop
into 11aa5f9 on nlohmann:develop.

[nlohmann]

Oh, I see we are not executing the test suite with disabled implicit conversions. I will take care of this.

We actually do check with in job ci_test_noimplicitconversions.

[nlohmann]

Looks good to me.

[nlohmann]

Will check #3859 after merging.

[nlohmann]

@theodelrieu AppVeyor complains: https://ci.appveyor.com/project/nlohmann/json/builds/51095672/job/s37fb0m5ggp1qyfc

[theodelrieu]

Seems that JSON_HAS_CPP_17 is defined, but MSVC doesn't set /std:c++17?
Shouldn't that flag be set when invoking cmake?

I don't have a Windows setup, so it'd be nice if a kind soul could take the burden.

[nlohmann]

In .github/external_ci/appveyor.yml, can you try changing

    - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
      configuration: Release
      platform: x86
      CXX_FLAGS: "/W4 /WX"
      CMAKE_OPTIONS: "-DJSON_ImplicitConversions=OFF"
      GENERATOR: Visual Studio 16 2019

to

    - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
      configuration: Release
      platform: x86
      CXX_FLAGS: "/permissive- /std:c++latest /utf-8 /W4 /WX"
      CMAKE_OPTIONS: "-DJSON_ImplicitConversions=OFF"
      GENERATOR: Visual Studio 16 2019

The AppVeyor build rules predate the ci.cmake scripts which should take care of setting all the flags properly.

[nlohmann]

Uff, AppVeyor is still failing... I will see if I can update the jobs there to the CI.cmake file. If I can't do this tomorrow, it will take some more time.

[github-actions]

This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions!

[gregmarr]

@nlohmann Has the AppVeyor build been fixed now?

[nlohmann]

No. I'll have a look at it. Appveyor has been pretty neglected over the years.

[github-actions]

This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions!

[gregmarr]

@nlohmann Three of the four edits here are obsolete. Only the change in the test file still applies.

[nlohmann]

(Last commit was wrong - I did more than conflict resolution.)