Add git detector to telemetry#2409
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 1.x #2409 +/- ##
============================================
+ Coverage 85.16% 85.18% +0.01%
- Complexity 21366 21395 +29
============================================
Files 1618 1620 +2
Lines 65951 66049 +98
============================================
+ Hits 56166 56262 +96
- Misses 9785 9787 +2 🚀 New features to boost your workflow:
|
|
@jdecool I really appreciate the PR but it's a bigger addition that should go through a Proposal Process since there are many architectural choices required in the implementation (like how to call git for example). We can keep this PR and work on it directly, but in the future I would appreciate opening a proper Proposal so we can first talk about how to implement the feature before writing any code |
|
Thanks for your review and your comments. No problem, I'm going to start the proposal process. To explain this PR, I've initially planned to try to add custom detectors dynamically during the PHPUnit process, but I think that git detector could be added by default. But you are right, it's a big change and should be discussed before implementing it. |
I fully agree, git detector should be part of the telemetry, it should be optional though, but also exposed to the whole telemetry not only phpunit bridge
I'm glad we are on the same page! |
|
@jdecool if you would like to brainstorm this first, or have any questions about implementation, scope etc, you can always find me on Flow discord: https://discord.gg/flow-php or you can schedule with me a session online |
|
Thanks @norberttech I've joined the discord. I plan to work on this PR and the proposal process this weekend. |
| @@ -40,6 +41,7 @@ public static function create(Configuration $config): Telemetry | |||
| { | |||
| $telemetryResource = resource_detector() | |||
There was a problem hiding this comment.
I would pass git_detector() here with other default detectors:
resource_detector([
os_detector(),
host_detector(),
process_detector(),
composer_detector(),
environment_detector(),
git_detector()
]);
but ideally (can be handled in next pr) we should probably allow users to configure which detectors they want to use
|
its looking good @jdecool ! |
|
Sorry, I haven't been around much lately; I was really busy last week. I'll finish this pull request soon. Thanks for your feedback and help. |
Take your time, it's a marathon, not a sprint. I always prefer to wait than merge something not fully baked. Thank you! |
This comment has been minimized.
This comment has been minimized.
jdecool
force-pushed
the
git-detector
branch
3 times, most recently
from
aca1187 to
d8c90c2
Compare
|
Thank you for your patience @norberttech . I've made some suggested changes; please take a look and let me know if it's OK for you. |
2 participants
Change Log
Added
Fixed
Changed
Removed
Deprecated
Security
Description
Sending PHPUnit data is very useful to see what happens during test execution.
But when used in an active project, where there are a lot of tests running, it's very interesting to have Git repository-related information.
So this PR adds a Git detector to attach some VCS information. As it could be interesting outside of the PHPUnit context, I've added it to the telemetry module.