Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DX] deprecated build shipped debug fuctions, allow using custom debug package instead; add local d() and dd() functions #4759

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

TomasVotruba
Copy link
Member

@TomasVotruba TomasVotruba commented Aug 10, 2023

Instead of shipping tracy with every Rector build, we should use debug tools only in dev code.

Not everyone wants to debug Rector, and not everyone want to use tracy :) we should let people use their own debug functions/tools, if they needed.

@TomasVotruba TomasVotruba changed the title deprecated build shipped debug fuctions, allow using custom debug package instead; add local d() and dd() functions [DX] deprecated build shipped debug fuctions, allow using custom debug package instead; add local d() and dd() functions Aug 10, 2023
"tomasvotruba/type-coverage": "^0.2",
"tomasvotruba/unused-public": "^0.2"
"tomasvotruba/unused-public": "^0.2",
"tracy/tracy": "^2.9"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all rector extensions will require-dev tracy/tracy then, or require back rector-debugging

https://github.com/rectorphp/rector-debugging

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware 👍 Let's add tracy/tracy if needed actually :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think require-dev rector/rector-debugging is the fastest way to avoid unnecesary add new functions.

Copy link
Member Author

@TomasVotruba TomasVotruba Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i want to keep dependency amount as low as possible. It's a common practice to let devs use the debugging packages/tools they need, when they need it.

Lets keep it as it is now = no changes in packages

Copy link
Member

@samsonasik samsonasik Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is add back rector/rector-debugging to require-dev into every rector-* extensions, and add here in require-dev as well.

require-dev is not shipped in rector build, but that will speed up dev

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue without debug tool in rector extensions, eg: rector-phpunit is, that it will cause contributors hard to debug the node, even us, that's why if it removed, we need to add back rector/rector-debugging to require-dev in every packages:

  • rector-src
  • rector-phpunit
  • rector-symfony
  • rector-doctrine
  • rector-downgrade-php

require-dev is not shipped in rector build, but that will speed up dev

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For note, https://github.com/rectorphp/rector-debugging is archived, if that will be used, that's need to be unarchive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood 👍 we can add tracy/tracy if needed to require-dev. It's good enough for quick dumping

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For note, rectorphp/rector-debugging is archived, if that will be used, that's need to be unarchive.

That's what I want to avoid → less dependencies :) That repository was not a good idea.

…kage instead; add local d() and dd() functions
@TomasVotruba TomasVotruba merged commit fa2a806 into main Aug 10, 2023
44 checks passed
@TomasVotruba TomasVotruba deleted the tv-tracy-less branch August 10, 2023 15:17
@TomasVotruba
Copy link
Member Author

Screenshot_20230810-172759

6 k lines less in the final release 🤙

@samsonasik
Copy link
Member

If we use rector-debugging in require-dev, that will not included in scoped vendor as well.

Using require-dev will still have less dependency to ship if you consider that

}
}


if (! function_exists('print_node')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print_node() call Dumper::dump($printedContent); that will error without tracy, the dump print content may just be replaced with var_dump

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants