debugbar logging requests using microtime by maniaba · Pull Request #5643 · codeigniter4/CodeIgniter4

maniaba · 2022-02-02T08:41:25Z

Feature request from the forum: https://forum.codeigniter.com/thread-81113.html

Each pull request should address a single issue and have a meaningful title.

Description
Explain what you have changed, and why.

Checklist:

Securely signed commits
Component(s) with PHPDoc blocks, only if necessary or adds value
Unit testing, with >80% coverage
User guide updated
Conforms to style guide

Feature request from the form: https://forum.codeigniter.com/thread-81113.html

kenjis · 2022-02-02T09:07:43Z

@maniaba Thank you for sending the PR.
You forgot GPG signing.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/signing.md

It ssems there are coding style violations and Static Analysis error.
See

Feature request from the form: https://forum.codeigniter.com/thread-81113.html

maniaba · 2022-02-02T09:23:11Z

@maniaba Thank you for sending the PR. You forgot GPG signing. See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/signing.md

It ssems there are coding style violations and Static Analysis error. See

https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#php-style

https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#static-analysis-on-php-code

I just made changes.

add GPG signing

kenjis · 2022-02-02T10:00:38Z

Your sign is unverified.

The email in this signature doesn’t match the committer email.

add GPG signing - verify email

sfadschm · 2022-02-02T19:03:55Z

+     * @param int   $limit   Max history files
     */
-    public function setFiles(int $current, int $limit = 20)
+    public function setFiles(float $current, int $limit = 20)


Do we need to break the signature here? I think we could just pass around the microtime as an int and then do number_format right before outputting.

This is BC break.

Given the limitations, I think it is acceptable to change the signature albeit a breaking change. Let's wait for other's opinions. @kenjis @MGatner ?

I don't mind (necessary) BC in minor version up if it is documented.
See https://forum.codeigniter.com/thread-80892-post-392846.html#pid392846

By the way, is float enough?
It seems only 4 digits like 1643882644.5667 in my PHP.

Maybe change this to a string, and make the file name look like "debugbar_{microtime} _ {session_value} .json" or add session_value to the contents of the file?
Then we will solve the request "when multiple developers work on different modules I suggest the option that developers can filter requests by session so that everyone can see it's own requests made by their browser."
I suggest adding a checkbox to the history tab, which would filter sessions by the browser.

That is a good refinement to the Debug Toolbar, but let us limit this PR to adding the microtime only. We can add the session and filtering in another PR.

I think type string is better. It is actually (a part of) filename, not time.

paulbalandan · 2022-02-03T01:52:50Z

+     * @param int   $limit   Max history files
     */
-    public function setFiles(int $current, int $limit = 20)
+    public function setFiles(float $current, int $limit = 20)


This is BC break.

paulbalandan · 2022-02-03T14:27:14Z

@maniaba Please add to the user guide changelog (1) debug toolbar is now using microtime instead of time, and (2) there is a possible BC break for those users extending the History Collector and they should probably update setFiles() method

kenjis · 2022-02-05T00:26:35Z

This is not (int):

CodeIgniter4/system/Debug/Toolbar.php

Lines 485 to 488 in 4ca2159

    
           $history->setFiles( 
        
               (int) Services::request()->getGet('debugbar_time'), 
        
               $this->config->maxHistory 
        
           );

kenjis · 2022-02-12T08:27:19Z

@maniaba
Rector check was failed. Can you fix?
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#static-analysis-on-php-code

And when you sync your repository, can you use git rebase?
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#pushing-your-branch

…o develop

kenjis · 2022-02-24T02:07:28Z

@maniaba
It seems you used git merge. Is it difficult to use git rebase?
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#pushing-your-branch

kenjis

The current implementation always shows 00 at the end of Datetime.
Microtime has 6 digits, so it is better to show 6 digits.

Co-authored-by: kenjis <kenji.uui@gmail.com>

kenjis · 2022-02-24T06:19:52Z

Your changes to the SCSS files did not match the expected CSS output.
diff --git a/system/Debug/Toolbar/Views/toolbar.css b/system/Debug/Toolbar/Views/toolbar.css
index 680ef52..11a3301 100644
--- a/system/Debug/Toolbar/Views/toolbar.css
+++ b/system/Debug/Toolbar/Views/toolbar.css
@@ -774,7 +774,8 @@
 }
 
 .debug-bar-width190p {
-  width: 190px;}
+  width: 190px;
+}
 
 .debug-bar-width20e {
   width: 20em;

https://github.com/codeigniter4/CodeIgniter4/runs/5314886769?check_suite_focus=true

kenjis · 2022-02-24T06:39:42Z

There was 1 failure:

1) CodeIgniter\Debug\Toolbar\Collectors\HistoryTest::testSetFiles
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'1645684473.268600'
+'1645684473.2686'

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Debug/Toolbar/Collectors/HistoryTest.php:75
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

kenjis · 2022-03-23T10:04:17Z

@maniaba
Fixing the failed tests seems to complete this PR.
Can you?

From codeigniter4#5643

kenjis · 2022-05-05T05:27:42Z

Closed via #5958

debugbar logging requests using microtime

69899fb

Feature request from the form: https://forum.codeigniter.com/thread-81113.html

debugbar logging requests using microtime

6d4f8e5

Feature request from the form: https://forum.codeigniter.com/thread-81113.html

maniaba added 3 commits February 2, 2022 10:52

debugbar logging requests using microtime

0e51d78

add GPG signing

debugbar logging requests using microtime

c10bd47

add GPG signing

debugbar logging requests using microtime

317d697

add GPG signing

kenjis added the enhancement PRs that improve existing functionalities label Feb 2, 2022

maniaba added 4 commits February 2, 2022 11:08

debugbar logging requests using microtime

58f6184

add GPG signing - verify email

fix datetime expects string, float given.

9abed8f

fix datetime expects string, float given.

4bc62cb

fix Strict comparison using

060c576

sfadschm reviewed Feb 2, 2022

View reviewed changes

paulbalandan reviewed Feb 3, 2022

View reviewed changes

paulbalandan added the breaking change Pull requests that may break existing functionalities label Feb 3, 2022

update comments and add assertSame for active row

b6b86ff

maniaba requested a review from paulbalandan February 3, 2022 08:33

paulbalandan reviewed Feb 3, 2022

View reviewed changes

test corrections

f2d55e1

paulbalandan removed the breaking change Pull requests that may break existing functionalities label Feb 3, 2022

user guide changelog

4ca2159

maniaba and others added 2 commits February 6, 2022 21:07

Change parameter method History::setFiles() type from float to string

4e13df7

Merge branch 'develop' into develop

329daac

Fix regex pattern

bf3263f

kenjis added the stale Pull requests with conflicts label Feb 21, 2022

Sync

8088318

maniaba and others added 7 commits February 23, 2022 19:55

Sync

acdaec1

Merge remote-tracking branch 'upstream/develop' into develop

4b97c22

update changelogs

10074e5

Merge branch 'develop' of https://github.com/maniaba/CodeIgniter4 int…

4c2ca8d

…o develop

Update v4.2.0.rst

fb8fa1a

Update v4.2.0.rst

3e47505

Merge branch 'develop' of https://github.com/maniaba/CodeIgniter4 int…

bff4c56

…o develop

kenjis added breaking change Pull requests that may break existing functionalities and removed stale Pull requests with conflicts labels Feb 24, 2022

kenjis reviewed Feb 24, 2022

View reviewed changes

Comment thread system/Debug/Toolbar.php Outdated

kenjis reviewed Feb 24, 2022

View reviewed changes

Comment thread system/Debug/Toolbar.php Outdated

kenjis reviewed Feb 24, 2022

View reviewed changes

Comment thread system/Debug/Toolbar/Collectors/History.php Outdated

kenjis requested changes Feb 24, 2022

View reviewed changes

maniaba and others added 4 commits February 24, 2022 07:11

Merge branch 'develop' into develop

070265c

Update system/Debug/Toolbar.php

cbfe5b4

Co-authored-by: kenjis <kenji.uui@gmail.com>

Update system/Debug/Toolbar/Collectors/History.php

4809c60

Co-authored-by: kenjis <kenji.uui@gmail.com>

Update system/Debug/Toolbar.php

282297c

Co-authored-by: kenjis <kenji.uui@gmail.com>

Update toolbar.css

c2b511a

kenjis added the stale Pull requests with conflicts label May 4, 2022

kenjis added a commit to kenjis/CodeIgniter4 that referenced this pull request May 4, 2022

feat: debugbar logging requests using microtime

eefc67b

From codeigniter4#5643

kenjis mentioned this pull request May 4, 2022

feat: Debugbar request microtime #5958

Merged

5 tasks

kenjis closed this May 5, 2022

Conversation

maniaba commented Feb 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kenjis commented Feb 2, 2022

Uh oh!

maniaba commented Feb 2, 2022

Uh oh!

kenjis commented Feb 2, 2022

Uh oh!

sfadschm Feb 2, 2022

Choose a reason for hiding this comment

Uh oh!

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

kenjis Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

maniaba Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

kenjis Feb 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulbalandan commented Feb 3, 2022

Uh oh!

kenjis commented Feb 5, 2022

Uh oh!

kenjis commented Feb 12, 2022

Uh oh!

kenjis commented Feb 24, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kenjis left a comment

Choose a reason for hiding this comment

Uh oh!

kenjis commented Feb 24, 2022

Uh oh!

kenjis commented Feb 24, 2022

Uh oh!

kenjis commented Mar 23, 2022

Uh oh!

kenjis commented May 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maniaba commented Feb 2, 2022 •

edited

Loading

maniaba Feb 3, 2022 •

edited

Loading

kenjis Feb 5, 2022 •

edited

Loading