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

Solved #174 and #214 #216

Closed
wants to merge 44 commits into from
Closed

Solved #174 and #214 #216

wants to merge 44 commits into from

Conversation

SamoylovMD
Copy link
Contributor

Now we can write tests according to junit 4 annotations syntax. Another feature is new equals(object, precision) method in Vector and Matrix which allows us to replace MockMatrix and MockVector classes safely.

*
* @return equals of this matrix to that
*/
public boolean equals(Object object, double precision);
Copy link
Owner

Choose a reason for hiding this comment

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

Can we make this method take matrix object instead? So, we have to move class casting to equals(object) method.

@vkostyukov
Copy link
Owner

Maxim @SamoylovMD, this is huge! I appreciate your hard work on a project. Let me carefully review it this evening (in 8-10 hours).

@SamoylovMD
Copy link
Contributor Author

Finally, I won those troubles with version conflict.

@vkostyukov
Copy link
Owner

43 commits? Omg!

@vkostyukov vkostyukov mentioned this pull request Dec 5, 2014
@sylvia43
Copy link
Contributor

sylvia43 commented Dec 5, 2014

Oh did I make this have merge conflicts :P

I only changed fields to final. It might be easier to merge this first then I'll redo what I did. Or we can manually merge this in (that's probably a better idea). I can help out with that.

@vkostyukov
Copy link
Owner

I would prefer somehow to clear this PR. I mean, it should not be 43 duplicated commits.

@sylvia43
Copy link
Contributor

sylvia43 commented Dec 5, 2014

The best solution there would be to squash all these commits. I'm guessing @SamoylovMD can do it, but I can help if necessary. Otherwise I can check this out, merge it with master, then @SamoylovMD can checkout my merged branch and update this PR.

(I actually really want to merge this. Call me crazy but I like merging/rebasing big things)

@vkostyukov
Copy link
Owner

Yeah. It's big. I'm not against its size: it's totally fine. I'm agains running the git log history.

@SamoylovMD
Copy link
Contributor Author

Oh crap. So, to clearify situation: what exactly do I need to do to merge upstream/master branch with my origin/master? And how to merge thousands of commits to one? Just say actions, I'm totally lost.
2014-12-06 02 13 08

@DWiechert
Copy link
Contributor

Here's a nice blog post on how to update your fork with the changes that have been merged into the main repo's master - http://joshbranchaud.com/blog/2014/01/17/Updating-Forked-Git-Repository-With-Latest-Upstream-Changes.html

@vkostyukov
Copy link
Owner

@SamoylovMD, you better have some sleep btw :) // looking at your commit log timestamps.

The only way I see is to create a separate branch, which is equal to current master and cherry-pick there all the needed commits from this PR.

@vkostyukov
Copy link
Owner

I can try to merge it using command line to avoid duplicates. @SamoylovMD you can live it as is for now.

@SamoylovMD
Copy link
Contributor Author

Ok, I'll just wait.

@sylvia43
Copy link
Contributor

sylvia43 commented Dec 6, 2014

I'll squash then manually merge it, then you can grab my copy. I'll get it done tonight.

@vkostyukov
Copy link
Owner

@anubiann00b, then you can just make a PR with @SamoylovMD' commits. This should work.

@sylvia43
Copy link
Contributor

sylvia43 commented Dec 6, 2014

Ok. I'll get on it tonight. This will be fun!

@sylvia43
Copy link
Contributor

sylvia43 commented Dec 6, 2014

Wait this will be a mess to squash... There are commits before and after the PR's I made. I'll just squash the contiguous blocks and the manually fix things in between.

@SamoylovMD
Copy link
Contributor Author

@DWiechert thx for the link!

@vkostyukov
Copy link
Owner

This is merged (thanks @anubiann00b!), so we can close this one.

@vkostyukov vkostyukov closed this Dec 6, 2014
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.

4 participants