[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
There are no open issues
b.stoop
b.stoop
b.stoop
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

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

  3. The issue has been resolved. Show all issues

    this is never returned

  4. 
      
b.stoop
b.stoop
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    This won't work

  3. 
      
b.stoop
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 4)
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    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. The issue has been resolved. Show all issues

    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    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. The issue has been resolved. Show all issues

    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)
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    We don't need that file :)

  6. 
      
b.stoop
b.stoop
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/MySQL/MySQLQueryResult.php (Diff revision 9)
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    shouldn't be public, make it protected

  3. The issue has been resolved. Show all issues

    Superfluous

  4. The issue has been resolved. Show all issues

    Not correct ;)

  5. The issue has been resolved. Show all issues

    Superfluous

  6. The issue has been resolved. Show all issues

    Not correct ;)

  7. 
      
b.stoop
b.stoop
pprkut
  1. 
      
  2. The issue has been resolved. Show all issues

    2021 :)

    1. It's just 2021 instead of 2012-2021?

    2. Yes :)

  3. The issue has been resolved. Show all issues

    2021 :)

  4. The issue has been resolved. Show all issues

    2021 :)

  5. 
      
b.stoop
pprkut
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...