Skip to content

Conversation

@degtyaryov
Copy link
Contributor

#12764

As requested, I did it for branch 8.1

…t be of type resource or null, PgSql\Connection given.
…t be of type resource or null, PgSql\Connection given.

Add test bugGH12763.phpt
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, just some nits.
It also should target the PHP-8.2 branch, although we can manually cherry-pick.

@@ -0,0 +1,23 @@
--TEST--
Bug #GH12763 (pg_untrace(): Argument #1 ($connection) must be of type resource or null, PgSql\Connection given)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: for GitHub bugs, use the gh12763.php as filename. Similarly the title should be GH-12763 (pg_untrace(): ...)
For bugs on the old bugtracker we use Bug #xxxxx title and bugxxxxx.phpt filename.


?>
--EXPECT--
OK No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Please include a newline at the end of phpt files.

@nielsdos nielsdos requested a review from devnexen November 27, 2023 12:32
@devnexen
Copy link
Member

devnexen commented Nov 27, 2023

Just advice for next time, do not pick releases branch such as 8.1.26, instead if you target 8.1 pick PHP-8.1.

…t be of type resource or null, PgSql\Connection given.

Rename test to gh12763.phpt. Add new line.
@devnexen
Copy link
Member

Thanks for the fix, just some nits. It also should target the PHP-8.2 branch, although we can manually cherry-pick.

That s fine we merge up usually anyway :)

Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

LGTM. I ll merge it later on.

@devnexen
Copy link
Member

Merged via 3f57bd80. Thanks.

@devnexen devnexen closed this Nov 27, 2023
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.

3 participants