Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed basic_json::diff and basic_json::patch not compiling with custom string type (#4134) #4137

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

tomalakgeretkal
Copy link
Contributor

Note that this has yet to be amalgamated (waiting for #4083).

@@ -36,10 +36,33 @@ class alt_string
alt_string(size_t count, char chr): str_impl(count, chr) {}
alt_string() = default;

template <typename...TParams>
alt_string& append(TParams&& ...params)
alt_string& append(std::size_t count, char ch)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template version broke SFINAE-driven feature-detection in detail/string_concat.hpp.

@tomalakgeretkal
Copy link
Contributor Author

Amalgamated.

@github-actions github-actions bot added L and removed M labels Sep 8, 2023
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update from the develop branch as it has a working CI now.

@nlohmann nlohmann added the please rebase Please rebase your branch to origin/develop label Sep 24, 2023
@coveralls
Copy link

coveralls commented Sep 25, 2023

Coverage Status

coverage: 100.0%. remained the same when pulling 96129bd on tomalakgeretkal:custom-string-fix into edffad0 on nlohmann:develop.

@github-actions
Copy link

🔴 Amalgamation check failed! 🔴

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

@tomalakgeretkal
Copy link
Contributor Author

Looks amalgamated here :( check-amalgamate works for me!

@gregmarr
Copy link
Contributor

You may be using a different version of astyle that produces different results. The check is using 3.1. We've seen that later versions produce different results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L please rebase Please rebase your branch to origin/develop tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants