Skip to content

Fix total count in flow tracker#7813

Merged
vjeux merged 1 commit intofacebook:masterfrom
vjeux:fix_total_count
Sep 27, 2016
Merged

Fix total count in flow tracker#7813
vjeux merged 1 commit intofacebook:masterfrom
vjeux:fix_total_count

Conversation

@vjeux
Copy link
Copy Markdown
Contributor

@vjeux vjeux commented Sep 26, 2016

When you put the output of a bash command in a variable, it replaces the \n with a space. Using ls instead of echo fixes it

Test Plan:
Run

ALL_FILES=`find src -name '*.js' | grep -v umd/ | grep -v __tests__ | grep -v __mocks__`
COUNT_ALL_FILES=`ls $ALL_FILES | wc -l`
echo $COUNT_ALL_FILES

Make sure that it outputs 221

@vjeux vjeux added this to the 15-next milestone Sep 26, 2016
@ghost ghost added the CLA Signed label Sep 26, 2016
@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented Sep 27, 2016

cc @zpao

Copy link
Copy Markdown
Member

@zpao zpao left a comment

Choose a reason for hiding this comment

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

I'll leave it up to you if you want to do what I suggested. Either way is fine.

Comment thread .travis.yml
./node_modules/.bin/grunt flow

ALL_FILES=`find src -name '*.js' | grep -v umd/ | grep -v __tests__ | grep -v __mocks__`
COUNT_ALL_FILES=`echo $ALL_FILES | wc -l`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

echo "$ALL_FILES" should work. I'd prefer that over ls as it's clearer intent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Omg, thanks! I couldn't figure out what I did wrong. Of course, sending it to the tokenizer of bash like this is going to interpret them as different arguments and be in one line when echo sees it! Good catch!

When you put the output of a bash command in a variable, it replaces the `\n` with a space. Using `ls` instead of `echo` fixes it

Test Plan:
Run

```
ALL_FILES=`find src -name '*.js' | grep -v umd/ | grep -v __tests__ | grep -v __mocks__`
COUNT_ALL_FILES=`ls $ALL_FILES | wc -l`
echo $COUNT_ALL_FILES
```

Make sure that it outputs 221
@vjeux vjeux merged commit 72ed5df into facebook:master Sep 27, 2016
@zpao zpao modified the milestones: 15-next, 15.4.0 Oct 4, 2016
zpao pushed a commit that referenced this pull request Oct 4, 2016
When you put the output of a bash command in a variable, it replaces the `\n` with a space. Using `ls` instead of `echo` fixes it

Test Plan:
Run

```
ALL_FILES=`find src -name '*.js' | grep -v umd/ | grep -v __tests__ | grep -v __mocks__`
COUNT_ALL_FILES=`ls $ALL_FILES | wc -l`
echo $COUNT_ALL_FILES
```

Make sure that it outputs 221
(cherry picked from commit 72ed5df)
acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
When you put the output of a bash command in a variable, it replaces the `\n` with a space. Using `ls` instead of `echo` fixes it

Test Plan:
Run

```
ALL_FILES=`find src -name '*.js' | grep -v umd/ | grep -v __tests__ | grep -v __mocks__`
COUNT_ALL_FILES=`ls $ALL_FILES | wc -l`
echo $COUNT_ALL_FILES
```

Make sure that it outputs 221
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants