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

Fixes for various old issues. #800

Merged
merged 14 commits into from Jan 13, 2018

Conversation

Projects
None yet
3 participants
@dkorolev
Member

dkorolev commented Jan 2, 2018

No description provided.

@dkorolev

This comment has been minimized.

Show comment
Hide comment
@dkorolev

dkorolev Jan 2, 2018

Member

В новый год -- с чистой совестью!

@mzhurovich @grixa @biodranik @sompylasar :)

Member

dkorolev commented Jan 2, 2018

В новый год -- с чистой совестью!

@mzhurovich @grixa @biodranik @sompylasar :)

@sompylasar

👍

Show outdated Hide outdated bricks/net/http/test.cc Outdated
@@ -33,8 +33,8 @@ SOFTWARE.
namespace current {
namespace serialization {
template <class JSON_FORMAT, typename T>
struct SerializeImpl<json::JSONStringifier<JSON_FORMAT>, std::vector<T>> {
template <class JSON_FORMAT, typename T, typename TA>

This comment has been minimized.

@sompylasar

sompylasar Jan 2, 2018

Contributor

Better names for T and TA?

@sompylasar

sompylasar Jan 2, 2018

Contributor

Better names for T and TA?

This comment has been minimized.

@dkorolev

dkorolev Jan 2, 2018

Member

T is canonical, and TA is, well, everyone knows it's an allocator.

@dkorolev

dkorolev Jan 2, 2018

Member

T is canonical, and TA is, well, everyone knows it's an allocator.

This comment has been minimized.

@sompylasar

sompylasar Jan 2, 2018

Contributor

http://en.cppreference.com/w/cpp/container/vector

template<
    class T,
    class Allocator = std::allocator<T>
> class vector;
@sompylasar

sompylasar Jan 2, 2018

Contributor

http://en.cppreference.com/w/cpp/container/vector

template<
    class T,
    class Allocator = std::allocator<T>
> class vector;

This comment has been minimized.

@dkorolev

dkorolev Jan 2, 2018

Member

И?

@dkorolev

dkorolev Jan 2, 2018

Member

И?

This comment has been minimized.

@sompylasar

sompylasar Jan 2, 2018

Contributor

That "everyone" can guess it's an allocator doesn't mean the names should require that guessing. The names are there to read them without extra looking around.

@sompylasar

sompylasar Jan 2, 2018

Contributor

That "everyone" can guess it's an allocator doesn't mean the names should require that guessing. The names are there to read them without extra looking around.

destination[i] = tmp;
}
} else if (!json::JSONPatchMode<JSON_FORMAT>::value || (json_parser && !json_parser.Current().IsArray())) {
CURRENT_THROW(JSONSchemaException("array", json_parser)); // LCOV_EXCL_LINE

This comment has been minimized.

@sompylasar

sompylasar Jan 2, 2018

Contributor

LCOV_EXCL_LINE -> LCOV_EXCL_START/END ?

@sompylasar

sompylasar Jan 2, 2018

Contributor

LCOV_EXCL_LINE -> LCOV_EXCL_START/END ?

This comment has been minimized.

@dkorolev

dkorolev Jan 2, 2018

Member

What for?

@dkorolev

dkorolev Jan 2, 2018

Member

What for?

This comment has been minimized.

@sompylasar

sompylasar Jan 2, 2018

Contributor

I don't know what for (maybe because the macro expands into more than one line), but you made the same change in this PR in another line.
screen shot 2018-01-02 at 12 52 38 pm

@sompylasar

sompylasar Jan 2, 2018

Contributor

I don't know what for (maybe because the macro expands into more than one line), but you made the same change in this PR in another line.
screen shot 2018-01-02 at 12 52 38 pm

This comment has been minimized.

@dkorolev

dkorolev Jan 2, 2018

Member

Most likely because the line was too long so I had to break it.

@dkorolev

dkorolev Jan 2, 2018

Member

Most likely because the line was too long so I had to break it.

@dkorolev

This comment has been minimized.

Show comment
Hide comment
@dkorolev

dkorolev Jan 4, 2018

Member

@mzhurovich, good to merge?

I've added a Semaphore badge, so far from dkorolev/Current, not C5T/Current, using the following snippet:

curl -L https://gist.githubusercontent.com/ervinb/e85fcb1e238199569dfb8452c6883177/raw/gcc-installer-semaphore.sh | bash
sudo apt-get update -y
sudo apt-get install -y clang
sudo apt-get install -y nasm
CPLUSPLUS=g++-5 make individual_tests
Member

dkorolev commented Jan 4, 2018

@mzhurovich, good to merge?

I've added a Semaphore badge, so far from dkorolev/Current, not C5T/Current, using the following snippet:

curl -L https://gist.githubusercontent.com/ervinb/e85fcb1e238199569dfb8452c6883177/raw/gcc-installer-semaphore.sh | bash
sudo apt-get update -y
sudo apt-get install -y clang
sudo apt-get install -y nasm
CPLUSPLUS=g++-5 make individual_tests

dkorolev added some commits Jan 11, 2018

Merge pull request #8 from dkorolev/flow_tool
Fixed header guards in examples.
Merge pull request #9 from dkorolev/flow_tool
"port" is now "uint16_t" in HTTP.

@grixa grixa merged commit c464db8 into C5T:stable Jan 13, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment