Skip to content
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

Submissions2 #116

Closed

Conversation

LaurensVoerman
Copy link
Contributor

fixing a few problems found on checking the clone functions.

@openscenegraph
Copy link
Owner

Hi Laurens,

I have just had a quick scan through the changes and I feel there are too wide ranging to be appropriate for a single pull request/merge. It would be far better to have each commit mapped to a single pull request as then I can merge the uncontroversial parts and where appropriate kick off a discussion about the parts that have questions over them.

For now I'll leave the pull request here, but only for reference, I'm not planning on merging it. Once we have the separated pull requests I'll work through them one by one.

@LaurensVoerman
Copy link
Contributor Author

Hi Robbert,
I will create the seperate pull requests, but I'm still working out how git
works, so this will take a little time.
Regards, Laurens.

On Thu, Aug 25, 2016 at 3:10 PM, OpenSceneGraph git repository <
notifications@github.com> wrote:

Hi Laurens,

I have just had a quick scan through the changes and I feel there are too
wide ranging to be appropriate for a single pull request/merge. It would be
far better to have each commit mapped to a single pull request as then I
can merge the uncontroversial parts and where appropriate kick off a
discussion about the parts that have questions over them.

For now I'll leave the pull request here, but only for reference, I'm not
planning on merging it. Once we have the separated pull requests I'll work
through them one by one.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#116 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AS5Z2mj-aO75A_WmvNjMyYpi8o9Xy65Oks5qjZQ3gaJpZM4Js0C1
.

@LaurensVoerman
Copy link
Contributor Author

Hi Robert,
I finished splitting this into separate pull requests, with one change in the new version:
not removing

BasicAnimationManager::BasicAnimationManager(const AnimationManagerBase& b, const osg::CopyOp& copyop)

because it's used in OpenSceneGraph/examples/osganimationviewer/AnimtkViewer.cpp

I suppose this pull request can now be rejected or deleted or something.
Regards, Laurens.

@openscenegraph
Copy link
Owner

Thanks Laurens, I'll now close this pull request, record of it will still be on github so if you do need to double check something it'll still be accessible.

@LaurensVoerman LaurensVoerman deleted the Submissions2 branch September 12, 2016 11:17
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.

None yet

2 participants