Skip to content

Conversation

@faridmovsumov
Copy link
Contributor

No description provided.

@GrahamCampbell
Copy link
Collaborator

Please use single quotes around string.

@GrahamCampbell
Copy link
Collaborator

Also, leave 2 blank lines between test methods.

@faridmovsumov
Copy link
Contributor Author

@GrahamCampbell thank you for your review I applied these changes.

@GrahamCampbell
Copy link
Collaborator

Please remove all trailing whitespace and squash to one commit.

@faridmovsumov
Copy link
Contributor Author

I research how can I squash to single commit I didn't do it before

@GrahamCampbell
Copy link
Collaborator

If it's going to be a problem, don't bother. It's not worth your time.

@faridmovsumov
Copy link
Contributor Author

I squashed my last 3 commits into a single

@GrahamCampbell
Copy link
Collaborator

Hmmm. There seems to be an issue with my browser displaying the page. Carry on...

@faridmovsumov
Copy link
Contributor Author

@GrahamCampbell are there any other problem?

@GrahamCampbell
Copy link
Collaborator

Not that I can see. You'll have to wait for @taylorotwell to review this.

@faridmovsumov
Copy link
Contributor Author

@GrahamCampbell thank you.

@vlakoff
Copy link
Contributor

vlakoff commented Aug 11, 2014

Ideally, lines of code should be ordered the same in both functions.

@feridmovsumov
Copy link

@taylorotwell are there any problem with this pr?

@GrahamCampbell
Copy link
Collaborator

@faridmovsumov Taylor can sometimes take a week or more to get around to reviewing some pulls. It may be better to ping him at the end of the month rather than a few days after sending this.

taylorotwell added a commit that referenced this pull request Aug 22, 2014
…str-class

Added some unit tests for quickRandom and random methods in  SupportStrTest.php
@taylorotwell taylorotwell merged commit dbe372d into laravel:4.2 Aug 22, 2014
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.

5 participants