search

perftools / xhgui

Web interface for XHProf profiling data can store data in MongoDB or PDO database
1.65k stars 342 forks source link

Feature: Implement watch methods for PDO driver #435

Closed fengqi closed 2 years ago

fengqi commented 2 years ago

Add saveWatch, getAllWatches, truncateWatches to PdoRepository:

glensc commented 2 years ago

you need to document the breaking change in the PR body.

fengqi commented 2 years ago

you need to document the breaking change in the PR body.

ok

fengqi commented 2 years ago

done

glensc commented 2 years ago

done

Well, the breaking change is that the table is no longer configurable via XHGUI_PDO_TABLE environment variable:

        'table' => getenv('XHGUI_PDO_TABLE') ?: 'results',
fengqi commented 2 years ago

done

Well, the breaking change is that the table is no longer configurable via XHGUI_PDO_TABLE environment variable:

        'table' => getenv('XHGUI_PDO_TABLE') ?: 'results',

change the db table name is not a routine operation, and mongo is not configured, so I hard-coded it in the __construct

glensc commented 2 years ago

I'm saying that the breaking change I described is not in PR body. it's breaking change for users who have relied on the environment variable.

fengqi commented 2 years ago

I see, this is indeed a problem, let me change it back. do you have any other suggestions on this?

glensc commented 2 years ago

@fengqi I'm saying you need to document it in the pull request body. you can of course restore the breaking change, so then it does not need to be documented.

fengqi commented 2 years ago

i have changed it back and i will submit other pdo implementation parts later

glensc commented 2 years ago

please consider doing atomic commits for further changes:

and if previous commit needs to be improved, git commit --fixup.

I'll squash/fixup myself your commits here before the merge.

fengqi commented 2 years ago

please consider doing atomic commits for further changes:

and if previous commit needs to be improved, git commit --fixup.

I'll squash/fixup myself your commits here before the merge.

got it, I pick all commit from the our used internally branch, so the modification was submitted all at once

glensc commented 2 years ago

I use git format-patch -1 COMMIT and git am 0001-foo.patch to transfer such commits. or if they are in same git repo (just different branches) can git cherry-pick COMMIT

fengqi commented 2 years ago

did i need to reopen a new pr？

glensc commented 2 years ago

@fengqi open new PR for what?

glensc commented 2 years ago

Fixing failing tests 0675a494abf072183e1e6d9765c3d2f02fba79c7: