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

Fix lint commands frozen on empty stdin#33523

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

Merged
nicolas-grekas merged 1 commit intosymfony:4.4fromchalasr:lint-layer
Sep 18, 2019

Conversation

@chalasr
Copy link
Member

QA
Branch?4.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

Running e.g.lint:yaml - with no piped content makes the command hangs currently, this makes it fail instead. Also fixes the command help which we forgot to update

@chalasrchalasr added this to thenext milestoneSep 9, 2019
@chalasrchalasrforce-pushed thelint-layer branch 2 times, most recently from8daae93 to06702edCompareSeptember 9, 2019 17:13
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedSep 9, 2019•
edited
Loading

This is wrong, hanging is normal behavior, the command waits for EOF, which is CTRL+D. That's exactly the same with any Unix command that reads from stdin.

The fact it works is relying on a race condition. Don't be fooled.

For 4.4 only, master shouldn't rely on ftell()

@chalasr
Copy link
MemberAuthor

Sure, but it's a behavior change which might give false positives in a CI. Take e.g:

cat unexisting| bin/console lint:yaml

On 4.3 the output is an error message and exit code is 1. On 4.4-dev it succeeds with code 0.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Oh I thought you submitted that again master. For 4.4 it works for me!

@chalasrchalasrforce-pushed thelint-layer branch 4 times, most recently from5f1ac9d to6472304CompareSeptember 10, 2019 04:17
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.

Much better imo, thanks!

@nicolas-grekas
Copy link
Member

Thank you@chalasr.

nicolas-grekas added a commit that referenced this pull requestSep 18, 2019
This PR was merged into the 4.4 branch.Discussion----------Fix lint commands frozen on empty stdin| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -Running e.g. `lint:yaml -` with no piped content makes the command hangs currently, this makes it fail instead. Also fixes the command help which we forgot to updateCommits-------b60e0c1 Fix lint commands frozen on empty stdin
@nicolas-grekasnicolas-grekas merged commitb60e0c1 intosymfony:4.4Sep 18, 2019
@chalasrchalasr deleted the lint-layer branchSeptember 18, 2019 16:47
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@ycerutoycerutoyceruto approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@chalasr@nicolas-grekas@stof@yceruto@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp