[Gravity] Integrate mysqli_get_warnings() in the result object for mysql/mariadb queries

Review Request #793 — Created Sept. 9, 2021 and submitted

b.stoop
Lunr
796
96d04bd...
lunr

Integrate mysqli_get_warnings() in the result object for mysql/mariadb queries

Wrote script to test if it's working locally and made unit tests

  • 0
  • 0
  • 16
  • 0
  • 16
Description From Last Updated
b.stoop
b.stoop
b.stoop
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    This looks like something you want to do once, not on every call to the method.

  3. this is never returned

  4. 
      
b.stoop
b.stoop
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     

    This won't work

  3. 
      
b.stoop
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 4)
     
     
     
     
     
     

    mixed is for example mysqli_warning|bool|int

    mysqli_warning|null is declared as ?mysqli_warning

  3. 
      
b.stoop
b.stoop
b.stoop
pprkut
  1. Looking good :)

    Minor tweaks here and there

  2. Try to be specific in your checks. We read ! as !== TRUE and while that checks out here, it's not immediately obvious. By being explicit you make it clear that we check a boolean value here.

  3. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    We like what we call "early exits". You can put a return statement inside the if. That way you can drop the else clause and save one indentation level for the do-while loop.

  4. use the helper method :)

    get_reflection_property_value()

    1. I dont think thats possible here, getValue() gets the property value from $this->result and get_reflection_property_value() can only get the property value of the reflection when that property is set manualy.

    2. That's fine. You can update the tests to follow that structure ;)

  5. test.php (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     

    We don't need that file :)

  6. 
      
b.stoop
b.stoop
pprkut
b.stoop
b.stoop
pprkut
b.stoop
pprkut
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...