Hi Serg, On Tue, Nov 02, 2021 at 02:19:05AM +0200, Sergei Krivonos wrote: > Applied fixes by your tests patch 638930f85f… > <https://github.com/MariaDB/server/commit/638930f85ff40bd39354da81e53e7278a4b28487> >
#1: Please check out the commit adding unit tests: http://lists.askmonty.org/pipermail/commits/2021-November/014773.html It turns out it is not that hard to add them. Any comments/ways to improve this? #2: In the above commit, note that two last unit tests show a scenario that is not currently checked. #3: What is the point of functions Json_writer::on_add_str(), Json_writer::on_start_array, Json_writer::on_start_object() ? I would understand if all the checking code was isolated in these functions. But it isn't. Please either remove the functions or explain the reasoning. #4 See the input below about the commit comment. Please change (feel free to force-push into the branch after that). I think this is all the input, and the patch will be ready for pushing once all of this is addressed. > > Item #2: please improve the commit comment as was requested in my previous > > e-mail: > > > > On Thu, Oct 28, 2021 at 04:58:35PM +0300, Sergey Petrunia wrote: > >> First, input on the commit comment. Please consider it as a request > >> applying > >> to ALL further commits to the MariaDB codebase. > >> > >> > >>> MDEV-23766: implemented requested debug checks > >> > >> Imagine somebody looking at this in a few years. Will they know what checks > >> were requested? They might get a clue by looking at the Jira text, but the > >> idea to make the commit comments concise and self-contained descriptions. > >> > >> Something like this: > >> > >> Line #1: one-line description of the patch: > >> > >>>> MDEV-23766: Make attempts to write invalid JSON cause assertion failures > >> > >> Subsequent lines: > >> > >>>> Make JSON writing API (class Json_writer) check the produced JSON > >>>> document > >>>> is valid. The following checks are made: > >>>> - JSON objects must contain named members > >>>> - JSON arrays must contain unnamed members. > >>>> - (TODO: add other restrictions we're enforcing). BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
