Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[FrameworkBundle] add --deprecations on debug:container command#32584

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@Simperfit
Copy link
Contributor

@SimperfitSimperfit commentedJul 17, 2019•
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#30089
LicenseMIT
Doc PRtodo

Two question for this:

  • is a table output enough ?
  • do we need to create a trait to share the code to the cache:warmup ? or maybe add a notice if there deprecation to check them with this command ?

Edit: new output
Capture d’écran de 2019-07-17 23-15-27

maxhelias reacted with thumbs up emoji
@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch 2 times, most recently frome504ade to2728a8dCompareJuly 17, 2019 17:07
@ycerutoyceruto added this to thenext milestoneJul 17, 2019
@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch 3 times, most recently from5f14174 toad4acacCompareJuly 17, 2019 17:49
Copy link
Member

@ycerutoyceruto left a comment

Choose a reason for hiding this comment

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

When the deprecation message is too large, the output is unreadable. Could we format that message? or do we have an option in the table helper to set the width of the column?

BTW very useful command, thanks!

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedJul 17, 2019•
edited
Loading

I would remove the table and take inspiration from the reports from the bridge if that makes sense.
Also, the name might be confusing: this is only about deprecations triggered during compile+warmup. There can be many more.

@Simperfit
Copy link
ContributorAuthor

Simperfit commentedJul 17, 2019 via email

I will check that ! Thanks.Maybe we could call it DebugContainerCompiledDeprecation (that’s big ;))Le mer. 17 juil. 2019 à 20:13, Nicolas Grekas <notifications@github.com> aécrit :
…
I would remove the table and take inspiration from the reports from the bridge if that makes sense. Also, the name might be confision: this is only about deprecations triggered during compile+warmup. There can be many more. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#32584?email_source=notifications&email_token=AA2KV4WMVDFWA6X4D66R5ZTP75OOLA5CNFSM4IESRN3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2GEDOY#issuecomment-512508347>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2KV4RMDLA4FXBFFRWAA3TP75OOLANCNFSM4IESRN3A> .

@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch 3 times, most recently fromed04eb3 tob21cc72CompareJuly 17, 2019 21:23
@Simperfit
Copy link
ContributorAuthor

@nicolas-grekas updated according to the bridge

@SimperfitSimperfit changed the title[FrameworkBundle] add debug:deprecations command[FrameworkBundle] add debug:warmed-deprecations commandJul 17, 2019
@SimperfitSimperfit changed the title[FrameworkBundle] add debug:warmed-deprecations command[FrameworkBundle] add debug:deprecations:warmed commandJul 18, 2019
@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch 3 times, most recently from4f156ea to43f18cdCompareJuly 18, 2019 11:48
@Simperfit
Copy link
ContributorAuthor

cc@yceruto WDYT ?

@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch from43f18cd toc27ad7aCompareJuly 23, 2019 06:50
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedJul 23, 2019•
edited
Loading

Wait, are deprecations triggered during warmup really added to Deprecations.log?

@Simperfit
Copy link
ContributorAuthor

Simperfit commentedJul 23, 2019•
edited
Loading

@nicolas-grekas I've taken thisfinally close into account in order to get the serialized data and the path of the file I wanted to use:

if ($this->debug &&true !==$previousHandler) {
restore_error_handler();
if (file_exists($this->deprecationLogsFilepath)) {
$previousLogs =unserialize(file_get_contents($this->deprecationLogsFilepath));
$collectedLogs =array_merge($previousLogs,$collectedLogs);
}
file_put_contents($this->deprecationLogsFilepath,serialize(array_values($collectedLogs)));

I think this is really the deprecation collected during cache warmup.

nicolas-grekas reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Oh, sorry I forgot about that :)
So, does this contains deprecations for container compilation too?

@Simperfit
Copy link
ContributorAuthor

Since we collect the same logs during container compilations, I think it does too

@nicolas-grekas
Copy link
Member

So do,debug:container --deprecations ?

Simperfit and B-Galati reacted with thumbs up emoji

@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch fromc27ad7a to77d5061CompareJuly 23, 2019 12:12
@SimperfitSimperfitforce-pushed thefeature/add-debug-deprecations-command branch from8ba1bda to6f00187CompareAugust 9, 2019 11:39
@Simperfit
Copy link
ContributorAuthor

PR rebased and comments fixed.

@Simperfit
Copy link
ContributorAuthor

cc@yceruto

@Simperfit
Copy link
ContributorAuthor

Simperfit commentedAug 14, 2019•
edited
Loading

PR updated and ready !

@fabpot
Copy link
Member

Why don't we supported all formats (JSON, Markdown and XML)?

@Simperfit
Copy link
ContributorAuthor

I don't see the need of getting this information in JSON for example ATM.

@fabpot
Copy link
Member

The support for many formats is for integration with third-party tools.

@fabpot
Copy link
Member

@Simperfit Do you have to finish this PR? We won't merge it if we don't support all formats. Or maybe someone else can take over to finish it, it should not be too time consuming.

@nicolas-grekasnicolas-grekasforce-pushed thefeature/add-debug-deprecations-command branch from4b62442 to9a97e33CompareSeptember 27, 2019 16:46
@nicolas-grekas
Copy link
Member

FYI, I rebased your branch
Small remaining details: when I run the command,the output shows
To search for a specific service, [...] at the end, that should be removed.

$formattedLogs = [];
$remainingCount =0;
foreach ($logsas$log) {
$formattedLogs[] =sprintf("%sx: %s\n in %s:%s",$log['count'],$log['message'],$log['file'],$log['line']);

Choose a reason for hiding this comment

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

we should use<href=%s>%s:%d</> here, to generate IDE links (which means using the FileLinkFormatter service to compute it)


/**
* Describes container deprecations.
*/
Copy link
Member

Choose a reason for hiding this comment

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

can be removed

$name =$this->findProperServiceName($input,$errorIo,$object,$name,$input->getOption('show-hidden'));
$options = ['id' =>$name];
}elseif ($input->getOption('deprecations')) {
$options = ['deprecations' =>true];
Copy link
Member

Choose a reason for hiding this comment

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

validateInput() needs to be updated to forbid combining this option with others.

@nicolas-grekasnicolas-grekas added the Help wantedIssues and PRs which are looking for volunteers to complete them. labelFeb 4, 2020
@fabpot
Copy link
Member

Any news here? Anyone willing to take over?

noemi-salaun reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Closing as stale, anyone willing to help, please start from this PR.

@noemi-salaun
Copy link
Contributor

I'll try implementing missing formats (md, json and xml)

nicolas-grekas added a commit that referenced this pull requestMar 13, 2020
…r command (Simperfit, noemi-salaun)This PR was merged into the 5.1-dev branch.Discussion----------[FrameworkBundle] add --deprecations on debug:container command| Q             | A| ------------- | ---| Branch?       | master (5.1)| Bug fix?      | no| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       |#30089  <!-- prefix each issue number with "Fix #", if any -->| License       | MIT| Doc PR        | todo_I apologize in advance, I am not fluent in English_Continuity of@Simperfit work from his PR#32584I added support for XML, JSON and markdown formats as well as unit tests for all formats.## XML format```xml<?xml version="1.0" encoding="UTF-8"?><deprecations remainingCount="5">  <deprecation count="3">    <message>Some deprecation message.</message>    <file>/path/to/some/file.php</file>    <line>39</line>  </deprecation>  <deprecation count="2">    <message>An other deprecation message.</message>    <file>/path/to/an/other/file.php</file>    <line>25</line>  </deprecation></deprecations>```## JSON format```json{  "remainingCount": 5,  "deprecations": [    {      "message": "Some deprecation message.",      "file": "\/path\/to\/some\/file.php",      "line": 39,      "count": 3    },    {      "message": "An other deprecation message.",      "file": "\/path\/to\/an\/other\/file.php",      "line": 25,      "count": 2    }  ]}```## Markdown format## Remaining deprecations (5)- 3x: "Some deprecation message." in /path/to/some/file.php:39- 2x: "An other deprecation message." in /path/to/an/other/file.php:25I added a new commit on top of@Simperfit 's one, but I don't know how you would like to have the git history writed.Commits-------ee6391e [FrameworkBundle] add all formats support for debug:container --deprecations command161f659 [FrameworkBundle] add --deprecations on debug:container command
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@chalasrchalasrchalasr requested changes

@ycerutoycerutoyceruto approved these changes

+2 more reviewers

@ro0NLro0NLro0NL approved these changes

@maxheliasmaxheliasmaxhelias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

FeatureFrameworkBundleHelp wantedIssues and PRs which are looking for volunteers to complete them.Status: Needs Work

Projects

None yet

Milestone

5.1

Development

Successfully merging this pull request may close these issues.

9 participants

@Simperfit@nicolas-grekas@fabpot@noemi-salaun@ro0NL@yceruto@chalasr@maxhelias@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp