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

Add getters for EventMotion properties. #104

Merged
merged 1 commit into from Mar 1, 2016

Conversation

Projects
None yet
4 participants
@hmeyer

hmeyer commented Feb 29, 2016

also add myself and my employer to AUTHORS.

@@ -17,8 +17,10 @@ Evgenii Pashkin <eapashkin@gmail.com>
Geoffrey French <frondit1985@gmail.com>
Gleb Kozyrev <gleb@gkoz.com>
Glenn Watson <gw@intuitionlibrary.com>
Google Inc.

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Feb 29, 2016

Member

I'm feeling uneasy with this...

@GuillaumeGomez

GuillaumeGomez Feb 29, 2016

Member

I'm feeling uneasy with this...

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Feb 29, 2016

Member

@hmeyer Could you please elaborate about the role of your employer here? It doesn't seem likely that two parties at once hold copyright for this patch.

On the technical side, the getters should borrow self (e.g. fn get_time(&self) -> u32) not consume it. We try to avoid using ffi type names in the higher level APIs, you can use it under the ::ModifierType name instead. The use gdk_ffi as ffi; line is not needed after #103.

Member

gkoz commented Feb 29, 2016

@hmeyer Could you please elaborate about the role of your employer here? It doesn't seem likely that two parties at once hold copyright for this patch.

On the technical side, the getters should borrow self (e.g. fn get_time(&self) -> u32) not consume it. We try to avoid using ffi type names in the higher level APIs, you can use it under the ::ModifierType name instead. The use gdk_ffi as ffi; line is not needed after #103.

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Feb 29, 2016

Member

The use gdk_ffi as ffi; line is not needed after #103.

Or rather, this needs a rebase.

Member

gkoz commented Feb 29, 2016

The use gdk_ffi as ffi; line is not needed after #103.

Or rather, this needs a rebase.

@hmeyer

This comment has been minimized.

Show comment
Hide comment
@hmeyer

hmeyer Mar 1, 2016

Disclaimer: I am not a lawyer.
But as I understand it, my employer holds the copyright for all my software development work. So that name needs to go into COPYRIGHT. I consequently removed my name.
Also I addressed the other comments.

hmeyer commented Mar 1, 2016

Disclaimer: I am not a lawyer.
But as I understand it, my employer holds the copyright for all my software development work. So that name needs to go into COPYRIGHT. I consequently removed my name.
Also I addressed the other comments.

@hmeyer

This comment has been minimized.

Show comment
Hide comment
@hmeyer

hmeyer Mar 1, 2016

Google employees participate in many open source projects, and Google Inc is consequently added to AUTHORS/COPYRIGHT. For example:
http://hg.mozilla.org/comm-central/file/tip/AUTHORS#l170

hmeyer commented Mar 1, 2016

Google employees participate in many open source projects, and Google Inc is consequently added to AUTHORS/COPYRIGHT. For example:
http://hg.mozilla.org/comm-central/file/tip/AUTHORS#l170

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Mar 1, 2016

Member

as I understand it, my employer holds the copyright for all my software development work.

I'm sad to hear this. If you don't own the code there's so far no reason for us to believe it has been open-sourced by the owner.

Member

gkoz commented Mar 1, 2016

as I understand it, my employer holds the copyright for all my software development work.

I'm sad to hear this. If you don't own the code there's so far no reason for us to believe it has been open-sourced by the owner.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Mar 1, 2016

Member

@hmeyer: Can you squash your commits please?

Member

GuillaumeGomez commented Mar 1, 2016

@hmeyer: Can you squash your commits please?

Show outdated Hide outdated src/event_motion.rs
Show outdated Hide outdated src/event_motion.rs
Add getters for EventMotion properties.
also add Google Inc. to COPYRIGHT.
@hmeyer

This comment has been minimized.

Show comment
Hide comment
@hmeyer

hmeyer Mar 1, 2016

Addressed the comments (and learned how to squash commits).
COPYRIGHT: There is a LICENSE file in the project which Google Inc. is aware of and to which it agrees. Think about this: If I had just added myself to COPRYRIGHT - what reason would you have to believe I have open-sourced m the patch?

hmeyer commented Mar 1, 2016

Addressed the comments (and learned how to squash commits).
COPYRIGHT: There is a LICENSE file in the project which Google Inc. is aware of and to which it agrees. Think about this: If I had just added myself to COPRYRIGHT - what reason would you have to believe I have open-sourced m the patch?

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Mar 1, 2016

Member

I think your employer has a policy and process for such cases designed to eliminate uncertainty. Saying that Google agrees to the license gives an impression of skirting that process.

Member

gkoz commented Mar 1, 2016

I think your employer has a policy and process for such cases designed to eliminate uncertainty. Saying that Google agrees to the license gives an impression of skirting that process.

@hmeyer

This comment has been minimized.

Show comment
Hide comment
@hmeyer

hmeyer Mar 1, 2016

Yes, there is a process. And yes, I went through that process and got approval.

hmeyer commented Mar 1, 2016

Yes, there is a process. And yes, I went through that process and got approval.

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Mar 1, 2016

Member

Great! Does that process allow you to paste here a copy of the approval or is it confidential?

Member

gkoz commented Mar 1, 2016

Great! Does that process allow you to paste here a copy of the approval or is it confidential?

@hmeyer

This comment has been minimized.

Show comment
Hide comment
@hmeyer

hmeyer Mar 1, 2016

The approval was: "LGTM"

hmeyer commented Mar 1, 2016

The approval was: "LGTM"

@gkoz

This comment has been minimized.

Show comment
Hide comment
@gkoz

gkoz Mar 1, 2016

Member

I guess that will do. Thanks!

Member

gkoz commented Mar 1, 2016

I guess that will do. Thanks!

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Mar 1, 2016

Member

Thanks a lot for your work @hmeyer!

Member

GuillaumeGomez commented Mar 1, 2016

Thanks a lot for your work @hmeyer!

GuillaumeGomez added a commit that referenced this pull request Mar 1, 2016

Merge pull request #104 from hmeyer/master
Add getters for EventMotion properties.

@GuillaumeGomez GuillaumeGomez merged commit db08aeb into gtk-rs:master Mar 1, 2016

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment