ilovesoup / hyracks

Automatically exported from code.google.com/p/hyracks
Apache License 2.0
0 stars 0 forks source link

Issue 104 Review #109

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Purpose of code changes on this branch:
Ensure IFrameWriter state transitions are maintained by all delegating IFWs.

When reviewing my code changes, please focus on:
Making sure that the state transitions are actually maintained :-)

Note: As I mentioned to Vinayak this morning, the changes here are not 
completely thorough. They do not cover all uses of IFrameWriters, but I've 
tried to cover all IFrameWriters that delegate to another IFW. Additionally, 
there are no test cases for this, but I still think that it is better than what 
we had before. I'm choosing to cut the "thorough-ness" short as I have more 
pressing issues to work on right now before the asterix release. I will file 
another issue to be looked at post-beta to finish this off.

Lastly, the changes tend to make the code a bit ugly and it seems there are 
similar patterns used everywhere (e.g. the changes mostly look like candidates 
for refactoring/abstraction), but I haven't found a nice way to do it. I'm all 
ears if you can think of a nicer way.

After the review, I'll merge this branch into:
master

 I'm not able to get Rietveld working still so unfortunately the changes are part of 2 commits with some *slight* overlap (changes in 2nd commit supersede the first):
https://code.google.com/p/hyracks/source/detail?r=343c29cbfe40a1f50f8a0f0ae45cd4
0403d07537&name=zheilbron/hyracks_issue104
https://code.google.com/p/hyracks/source/detail?r=fa2a602d5dae63f15e5cc4802125f1
34e54f1863&name=zheilbron/hyracks_issue104

Original issue reported on code.google.com by zheilb...@gmail.com on 24 May 2013 at 4:48

GoogleCodeExporter commented 9 years ago
Removing this since it's way old.
Will revisit in the future.

Original comment by zheilb...@gmail.com on 30 Jan 2014 at 7:17