debugbar logging requests using microtime #5643

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

system/Debug/Toolbar/Collectors/History.php

     */
-    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.

system/Debug/Toolbar.php

system/Debug/Toolbar/Collectors/History.php

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

system/Debug/Toolbar/Collectors/History.php

     */
-    public function setFiles(int $current, int $limit = 20)
+    public function setFiles(float $current, int $limit = 20)


This is BC break.

system/Debug/Toolbar/Collectors/History.php

tests/system/Debug/Toolbar/Collectors/HistoryTest.php

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

system/Debug/Toolbar.php

system/Debug/Toolbar/Collectors/History.php

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

system/Debug/Toolbar.php Outdated Show resolved Hide resolved

kenjis reviewed Feb 24, 2022

View reviewed changes

system/Debug/Toolbar.php Outdated Show resolved Hide resolved

kenjis reviewed Feb 24, 2022

View reviewed changes

system/Debug/Toolbar/Collectors/History.php Outdated Show resolved Hide resolved

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

debugbar logging requests using microtime #5643

debugbar logging requests using microtime #5643

maniaba commented Feb 2, 2022 •

edited

Loading

kenjis commented Feb 2, 2022

maniaba commented Feb 2, 2022

kenjis commented Feb 2, 2022

sfadschm Feb 2, 2022

paulbalandan Feb 3, 2022

paulbalandan Feb 3, 2022

kenjis Feb 3, 2022

maniaba Feb 3, 2022 •

edited

Loading

paulbalandan Feb 3, 2022

kenjis Feb 5, 2022 •

edited

Loading

paulbalandan Feb 3, 2022

paulbalandan commented Feb 3, 2022

kenjis commented Feb 5, 2022

kenjis commented Feb 12, 2022

kenjis commented Feb 24, 2022

kenjis left a comment

kenjis commented Feb 24, 2022

kenjis commented Feb 24, 2022

kenjis commented Mar 23, 2022

kenjis commented May 5, 2022

debugbar logging requests using microtime #5643

debugbar logging requests using microtime #5643

Conversation

maniaba commented Feb 2, 2022 • edited Loading

kenjis commented Feb 2, 2022

maniaba commented Feb 2, 2022

kenjis commented Feb 2, 2022

sfadschm Feb 2, 2022

Choose a reason for hiding this comment

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

kenjis Feb 3, 2022

Choose a reason for hiding this comment

maniaba Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

kenjis Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

paulbalandan Feb 3, 2022

Choose a reason for hiding this comment

paulbalandan commented Feb 3, 2022

kenjis commented Feb 5, 2022

kenjis commented Feb 12, 2022

kenjis commented Feb 24, 2022

kenjis left a comment

Choose a reason for hiding this comment

kenjis commented Feb 24, 2022

kenjis commented Feb 24, 2022

kenjis commented Mar 23, 2022

kenjis commented May 5, 2022

maniaba commented Feb 2, 2022 •

edited

Loading

maniaba Feb 3, 2022 •

edited

Loading

kenjis Feb 5, 2022 •

edited

Loading