Skip to content

Conversation

brain5lug
Copy link

Copy link
Collaborator

@drewdzzz drewdzzz left a comment

Choose a reason for hiding this comment

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

Thank you for the patch and for your interest in our project! I have no objections except for one nitpick.

@drewdzzz
Copy link
Collaborator

@CuriousGeorgiy I've checked linter complaints - they don't relate to the actual code changes, so I suggest to ignore it. The only actual complaints are about writing all the move constructors from the second commit in one line - that's what my nitpick suggests.

@drewdzzz drewdzzz assigned brain5lug and unassigned drewdzzz Oct 10, 2025
@CuriousGeorgiy CuriousGeorgiy removed their assignment Oct 11, 2025
@brain5lug brain5lug force-pushed the brain5lug/fixes-pack branch from 0401fe1 to 320af2d Compare October 13, 2025 15:51
Eliminate copying from std::stringstream if complied with C++20 or higher
std::forward were replace std::move for fixed type context where
it does not make sense. Some redundant comment were removed as they
declare obvious actions under them.
For most of cases encoding fuction such as `call`, `execute` and
`prepare` use predefined fixed literal. Those values are best to
be expressed as std::string_view literals.
std::string_view are really usefull and can be easily createad
from any sequense memory areas.
There are no changes are required from cliend side as
std::string_view can be implicilty coverted from std::string.
@brain5lug brain5lug force-pushed the brain5lug/fixes-pack branch from 320af2d to 4fecfc2 Compare October 13, 2025 16:27
@CuriousGeorgiy CuriousGeorgiy assigned drewdzzz and unassigned brain5lug Oct 13, 2025
Copy link
Member

@CuriousGeorgiy CuriousGeorgiy left a comment

Choose a reason for hiding this comment

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

@brain5lug
Thanks for the patches and your fixes, all neat and shiny! Happy to see you around and chat about tntcxx code and design if you have any more ideas for improvement.

@drewdzzz drewdzzz merged commit 0979e94 into tarantool:master Oct 14, 2025
95 of 97 checks passed
@brain5lug brain5lug deleted the brain5lug/fixes-pack branch October 14, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants