Skip to content

Conversation

FridaTveit
Copy link
Contributor

Issue #41.

@oskopek
Copy link
Owner

oskopek commented Jan 9, 2017 via email

@coveralls
Copy link

coveralls commented Jan 9, 2017

Coverage Status

Coverage remained the same at 14.463% when pulling 1189a12 on FridaTveit:RemoveUnneededThisinHoughTransformation into fde1d42 on oskopek:master.

@FridaTveit
Copy link
Contributor Author

No problem :)

Copy link
Owner

@oskopek oskopek left a comment

Choose a reason for hiding this comment

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

Apart from the two comments, this is ready to merge. Thanks!

this.bitmap = new float[width][height];
maxPoint = null;
bitmap = new float[width][height];
this.width = width;
Copy link
Owner

Choose a reason for hiding this comment

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

Lets make width and height final. I know this wasn't the aim of your PR, but it will be way easier to reason about all future changes this way.

Copy link
Owner

Choose a reason for hiding this comment

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

Also, bitmap.

@oskopek
Copy link
Owner

oskopek commented Jan 13, 2017

Merging now, I'll apply the changes myself.

@oskopek oskopek merged commit 9cd4053 into oskopek:master Jan 13, 2017
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.

3 participants