Skip to content

 #2114

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.

Sign up for GitHub

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

Jump to bottom
Open
absolutelyNoWarranty wants to merge 1 commit into
base: master
Choose a base branch
from
Open

 #2114

absolutelyNoWarranty wants to merge 1 commit into from

Conversation

@absolutelyNoWarranty
Copy link

@absolutelyNoWarranty absolutelyNoWarranty commented Mar 19, 2018 •
edited
Loading

No description provided.

@absolutelyNoWarranty absolutelyNoWarranty changed the title ⠀  Mar 19, 2018
@stegben
Copy link

stegben commented Mar 19, 2018

LGTM

1 similar comment
@kpman
Copy link

kpman commented Mar 19, 2018

LGTM

@SoluMilken
Copy link

SoluMilken commented Mar 19, 2018

太神拉 ～～～～～

@wtflink
Copy link

wtflink commented Mar 19, 2018

.

@hoaaoh
Copy link

hoaaoh commented Mar 19, 2018

LGTM

@eshuang88
Copy link

eshuang88 commented Mar 19, 2018

LGTM

@angelo-moreira
Copy link

angelo-moreira commented Mar 19, 2018

frnhr
frnhr suggested changes Mar 19, 2018
View reviewed changes
Copy link

@frnhr frnhr left a comment

Choose a reason for hiding this comment

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

This is not very readable code:

Please refactor to something like this instead:

holykol
holykol approved these changes Mar 21, 2018
View reviewed changes
Copy link

@holykol holykol left a comment

Choose a reason for hiding this comment

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

Looks nice!

@jhakala
Copy link

jhakala commented Mar 25, 2018

@mszb
Copy link

mszb commented Mar 29, 2018

add this code snippet for performance improvement

@galpratama
Copy link

galpratama commented Apr 10, 2018

 

@isurupalliyaguru
Copy link

isurupalliyaguru commented Apr 16, 2018

After those I think this looks fine

Purpzie
Purpzie suggested changes Apr 16, 2018
View reviewed changes
Copy link

@Purpzie Purpzie left a comment

Choose a reason for hiding this comment

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

Everything looks fine, though you should probably fix this line.

Here is a much better version.

@drepram
Copy link

drepram commented Apr 26, 2018

lgtm.

@DrSensor
Copy link

DrSensor commented Apr 27, 2018

Can we merge this?

@NSLog0
Copy link

NSLog0 commented May 3, 2018

Since this PR was no title I'm wondering how are you guys click into this PR?

@joaoprp
Copy link

joaoprp commented May 3, 2018

@Avatarr There's a pattern /pull/{id}. Knowing the id, you can access the PR, for the first time.

Then, as soon as anyone comments on it, it gets easy as a link to comments will appear.

@garrettgsb
Copy link

garrettgsb commented May 17, 2018

Also you can click the comment icon on the right. 🤔

@jhakala
Copy link

jhakala commented May 17, 2018

TSIA

eponymz
eponymz approved these changes Aug 25, 2018
View reviewed changes
Copy link

@eponymz eponymz left a comment

Choose a reason for hiding this comment

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

This should definitely be merged. Would make runtime errors obsolete.

@meyyappanv
Copy link

meyyappanv commented Jan 10, 2019

LGTM

@jmdejong
Copy link

jmdejong commented Apr 3, 2019 •
edited
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

9 more reviewers

@frnhr frnhr frnhr requested changes

@Purpzie Purpzie Purpzie requested changes

@joaoprp joaoprp joaoprp approved these changes

@soullivaneuh soullivaneuh soullivaneuh approved these changes

@Aquaj Aquaj Aquaj approved these changes

@ariejoseph ariejoseph ariejoseph approved these changes

@holykol holykol holykol approved these changes

@eponymz eponymz eponymz approved these changes

@neal2018 neal2018 neal2018 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.