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

Re-requesting, with much simplified changes #5

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@BrianRossmajer

BrianRossmajer commented Jan 22, 2015

I agree with your points, here I've removed the extra POD and just renamed the internal functions with an underscore. Updated the version number because new() can't be called as a instance method anymore.

I tried a rebase to squash my changes, but I don't think it worked properly as they had already been pushed to github. If there's a way to merge the final change without the intermediate ones, that would be preferable, I think.

Oh -- I left determinant as a public method, meant to talk to you about that first; I only did that because it does have geometric meaning that could be useful.

Thanks, Marc!

BrianRossmajer added some commits Jan 19, 2015

Removed ability to call new as instance method, as per suggestion here:
http://www.stonehenge.com/merlyn/UnixReview/col52.html
since we already have a proper clone method and the original way
is hard to Devel::Cover, this seemed logical.
Changed matrix_multiply to an instance method, renamed it with a lead…
…ing underscore to indicate internal only use
Changed matrix_multiply to an instance method, renamed it with a lead…
…ing underscore to indicate internal only use
added POD documentation for determinant()
Made some methods internal by preceding underscore
Changed matrix_multiply to an instance method, renamed it with a leading underscore to indicate internal only use
Incremented version number due to changing how new() works and one internal method calling change
No changes to the math were done, only certain calls and function names
Merge branch 'master' of https://github.com/BrianRossmajer/perl-modules
Conflicts:
	Geometry-AffineTransform/lib/Geometry/AffineTransform.pm
	Geometry-AffineTransform/t/lib/Geometry/AffineTransform/Test.pm

liyanage added a commit that referenced this pull request Jan 22, 2015

Squash commit of #5
    added POD documentation for determinant()
    Made some methods internal by preceding underscore
    Changed matrix_multiply to an instance method, renamed it with a leading underscore to indicate internal only use
    Incremented version number due to changing how new() works and one internal method calling change
    No changes to the math were done, only certain calls and function names

    Removed ability to call new as instance method, as per suggestion here:
    http://www.stonehenge.com/merlyn/UnixReview/col52.html
    since we already have a proper clone method and the original way
    is hard to Devel::Cover, this seemed logical.
@liyanage

This comment has been minimized.

Show comment
Hide comment
@liyanage

liyanage Jan 22, 2015

Owner

Squash-merged in e333330.

Thanks!

Owner

liyanage commented Jan 22, 2015

Squash-merged in e333330.

Thanks!

@liyanage liyanage closed this Jan 22, 2015

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