Skip to content

Conversation

@doutriaux1
Copy link
Contributor

add to let update know to use CellData rather than PointData
code was in place but was returning False rather than True in this instance
fix #1234
still need to add tests
add to let update know to use CellData rather than PointData
code was in place but was returning False rather than True in this instance
fix #1234
still need to add tests
Copy link
Contributor

Choose a reason for hiding this comment

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

These map entries really need some documentation. I have no idea what this change should do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes but aren't we changing all of this anyway once we refactor?

@alliepiper
Copy link
Contributor

Building/testing now.

@aashish24
Copy link
Contributor

hey guys.. not picking on anyone but please use better short messages for the commit.

@aashish24
Copy link
Contributor

@dlonie I am assuming that you will have some feedback on it.

@alliepiper
Copy link
Contributor

Yep, still building. Assuming all goes well this can go in -- we can work on documenting that map when the animation code gets hit by the refactoring sweep.

@aashish24
Copy link
Contributor

👍 in general we should document our code as much as possible atleast when something is not trivial or error prone. In my document, I will make it a requirement.

@alliepiper
Copy link
Contributor

LGTM, tests pass.

@alliepiper alliepiper closed this Apr 28, 2015
@alliepiper alliepiper reopened this Apr 28, 2015
alliepiper pushed a commit that referenced this pull request Apr 28, 2015
@alliepiper alliepiper merged commit 29a1e60 into master Apr 28, 2015
@alliepiper alliepiper deleted the issue_1234_boxfill_anim_broken branch April 28, 2015 14:30
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