Implement pdo: getForUrl(), getPercentileForUrl(), getAll() by fengqi · Pull Request #436 · perftools/xhgui

@fengqi

@glensc

@fengqi something wrong here. I've seen already these commits. also, try to make PR title more descriptive. also use Draft if the PR is not ready for merge, as it seems you have same commits here that are in the previous PR, so the previous one needs to be merged first so this can be rebased.

@fengqi fengqi marked this pull request as draft

November 8, 2021 03:17

glensc

glensc

glensc

glensc

glensc

glensc

@fengqi fengqi changed the title Implement pdo Implement pdo: getForUrl(), getPercentileForUrl(), getAll()

Nov 10, 2021

@glensc

@fengqi please don't force push during review. use git fixup, otherwise I do not see differences with the previous changes, and have to do review from scratch.

you can rebase and force push after you've pushed fixup commit

also, leave note of each fix if it's part of the discussion, like:

  • "fixed in COMMIT_HASH_OF_FIXUP_COMMIT"

@glensc

@fengqi

please don't force push during review. use git fixup, otherwise I do not see differences with the previous changes, and have to do review from scratch.

you can rebase and force push after you've pushed fixup commit

also, leave note of each fix if it's part of the discussion, like:

ok

@fengqi

@fengqi

The recent commit are to fix the CS-Fixer error😂

@fengqi

@glensc Thank you for your reminder, I rarely contribute code publicly on Github, so many rules are not understood.
I have learned a lot by contributing code this time

@fengqi fengqi marked this pull request as ready for review

November 10, 2021 07:17

@glensc

@fengqi I think you've finished here (CI passes), so you can do the final rebase. or do you have anything else in todo?

@glensc

also, while rebasing, update branch from origin:

This branch is out-of-date with the base branch
Merge the latest changes from 0.19.x into this branch.

GitHub suggests creating a merge commit, but in this repo, we prefer to rebase.

@fengqi

@fengqi

@glensc There is nothing todo, the rebase is complete

glensc

@glensc

@fengqi

There are few more ✗'s in #320, if that's what you were attempting to solve here:

Searcher::getAll(direction) should already solved, others are used internally in our low frequency, i will solve it in my free time.

@glensc

Updated the table, marked that hardcoded direction (#337) was solved via #436

@fengqi

Updated the table, marked that hardcoded direction (#337) was solved via #436

ok