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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Gradient trait for RadialGradient #83

Merged
merged 1 commit into from Jul 17, 2016

Conversation

Projects
None yet
4 participants
@Jayshua
Contributor

Jayshua commented Jul 16, 2016

I could be wrong about this, maybe I'm just missing something. Shouldn't the Gradient trait be implemented for RadialGradient?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 16, 2016

Member

👍

Member

EPashkin commented Jul 16, 2016

👍

@@ -245,6 +245,8 @@ impl RadialGradient {
}
}
impl Gradient for RadialGradient{}

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Jul 16, 2016

Member

Extra empty new line.

@GuillaumeGomez

GuillaumeGomez Jul 16, 2016

Member

Extra empty new line.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 16, 2016

Member

Thanks!

Member

GuillaumeGomez commented Jul 16, 2016

Thanks!

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Jul 16, 2016

Member

👍

Member

gkoz commented Jul 16, 2016

👍

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 16, 2016

Member

So everyone is ok with your change. Just remove the extra empty line, squash the extra commit and we're all good!

Member

GuillaumeGomez commented Jul 16, 2016

So everyone is ok with your change. Just remove the extra empty line, squash the extra commit and we're all good!

@Jayshua

This comment has been minimized.

Show comment
Hide comment
@Jayshua

Jayshua Jul 16, 2016

Contributor

Not that I have a problem with removing the extra line, but I was trying to match the formatting already there and just under the LinearGradient code. Are you sure it shouldn't be there?

Contributor

Jayshua commented Jul 16, 2016

Not that I have a problem with removing the extra line, but I was trying to match the formatting already there and just under the LinearGradient code. Are you sure it shouldn't be there?

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 16, 2016

Member

Not sure about this...

@gkoz: ?

Member

GuillaumeGomez commented Jul 16, 2016

Not sure about this...

@gkoz: ?

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Jul 17, 2016

Member

I'll support @Jayshua on this because following the apparent style is good and it feels uncool to punish a contributor for someone's peculiar choice of style some years ago ;)

Member

gkoz commented Jul 17, 2016

I'll support @Jayshua on this because following the apparent style is good and it feels uncool to punish a contributor for someone's peculiar choice of style some years ago ;)

@gkoz gkoz merged commit a4a8e95 into gtk-rs:master Jul 17, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 17, 2016

Member

@gkoz: I just thought it was a typo. Don't want to punish anyone haha.

@Jayshua: Thanks for the PR and sorry for the delay!

Member

GuillaumeGomez commented Jul 17, 2016

@gkoz: I just thought it was a typo. Don't want to punish anyone haha.

@Jayshua: Thanks for the PR and sorry for the delay!

@Jayshua

This comment has been minimized.

Show comment
Hide comment
@Jayshua

Jayshua Jul 17, 2016

Contributor

No problem, glad I could help!

Contributor

Jayshua commented Jul 17, 2016

No problem, glad I could help!

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