Refactor Fable #227
Merged
Refactor Fable #227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This follows the pattern set by other libraries, such as Boost and nlohmann/json.
This makes it clearer that it should be included after everything else, and if someone sorts the schema commits by accident, this will preserve that.
BREAKING CHANGE: This commit renames a public header file and
requires that rename your #include statements:
- #include <fable/json/with_eigen.hpp>
+ #include <fable/utility/eigen.hpp>
BREAKING CHANGE: This commit renames a public header file and
requires that rename your #include statements:
- #include <fable/json/with_std.hpp>
+ #include <fable/utility/memory.hpp>
….hpp
BREAKING CHANGE: This commit renames a public header file and
requires that you rename your #include statements:
- #include <fable/json/with_boost.hpp>
+ #include <fable/utility/boost_optional.hpp>
cassava
force-pushed
the
ben/fable-refactor
branch
3 times, most recently
from
4bdc61e to
d0f9ed5
Compare
cassava
force-pushed
the
ben/fable-refactor
branch
2 times, most recently
from
e49b853 to
3b5cd7a
Compare
cassava
force-pushed
the
ben/fable-refactor
branch
from
f39aff2 to
6bfee25
Compare
clonker
reviewed
Apr 10, 2024
cassava
added
the
breaking change
Add serialization to and deserialization from strings in a manner as exact as possible.
BREAKING CHANGE: If you want to use boost::optional, you now need to include `fable/schema/boost_optional.hpp`.
…esystem::path BREAKING CHANGE: If you want to use boost::filesystem::path, you now need to include `fable/schema/boost_path.hpp`.
This is orthogonal with the standard library (std::vector) and allows us to add the Array type, which corresponds to the std::array type. BREAKING CHANGE: If you used fable::schema::Array directly you need to rename it to fable::schema::Vector. This is especially important as a future commit will introduce a separate Array schema type.
BREAKING CHANGE: Packages that relied on fable to provide Boost as
a transitive dependency need to add it to their project themselves:
For example, with CMake:
```cmake
find_package(Boost COMPONENTS headers filesystem REQUIRED)
target_link_libraries(${yourTarget} PUBLIC Boost::headers Boost::filesystem)
```
And with Conan:
```python
\# class ConanFile:
def requirements(self):
self.requires("boost/1.78")
```
This has a few advantages: 1. Users know where types come from, which aids them in finding documentation and further information. It encourages them to use other facilities from the library. The fable types are not an implementation detail. 2. It allows selective header inclusion and use of forward declarations to cut down compile times.
This was also a clang-tidy finding.
It appears the bug has been resolved for some time. (The nature of the bug prevented compilation, so if it compiles, it should work.)
Also improve the name from f to deserialize_fn.
- Use /* unused */ for unused parameters - Use clearer argument names - Add NOLINT comments where behavior is as-intended - Arrange methods consistently - Format code consistently
This simplifies the code as well as providing a more efficient implementation for string instantiation.
This requires you to include fable/utility/sol.hpp and provide the sol2 library yourself.
SOL_ALL_SAFETIES_ON sets SOL_SAFE_NUMERICS, so setting the latter is redundant. In the nlohmann to_json conversion from Lua tables, we ensure that one of those two values is set.
This happened when incorrectly casting from signed to unsigned. Tests have been added to test these edge cases.
It turned out to not be a problem with Environment as a problem with the test and/or documentation. Resolves issue #222.
During writing tests for the documentation, some bugs related to the use of JSON pointer in error messages were found and are corrected in this commit.
clonker
approved these changes
Apr 20, 2024
cassava
force-pushed
the
ben/fable-refactor
branch
from
6a83592 to
1338e39
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This has so many changes I know I'm going to have a lot of work ahead of me writing what has all changed.
I have tried to put each change in its own commit for easier review.
Each commit has been compiled and tested individually.