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

Rename coGroupDataSet.scala to CoGroupDataSet.scala, and crossDataSet.scala to CrossDataSet.scala #324

Closed
wants to merge 2 commits into from
Closed

Rename coGroupDataSet.scala to CoGroupDataSet.scala, and crossDataSet.scala to CrossDataSet.scala #324

wants to merge 2 commits into from

Conversation

hsaputra
Copy link
Contributor

This PR contains changes to follow Scala style:
-) Rename coGroupDataSet.scala to CoGroupDataSet.scala, and crossDataSet.scala to CrossDataSet.scala
-) Move the UnfinishedCoGroupOperation class into its own Scala file

The UnfinishedCoGroupOperation does not relate closely to CoGroupOperation
via sealed modifier so per Scala style guide [1] I propose to move it to
separate file.

[1] http://docs.scala-lang.org/style/files.html
@hsaputra
Copy link
Contributor Author

FYI @aljoscha

@aljoscha
Copy link
Contributor

Why do you want to move them?
On Jan 20, 2015 4:42 AM, "Henry Saputra" notifications@github.com wrote:

FYI @aljoscha https://github.com/aljoscha


Reply to this email directly or view it on GitHub
#324 (comment).

@hsaputra
Copy link
Contributor Author

I rename the file from coGroupDataSet.scala to CoGroupDataSet.scala and crossDataSet.scala to CrossDataSet.scala to follow convention Scala file naming.

And move out UnfinishedCoGroupOperation class because it is a high level public class by itself and not dependent on CoGroupOperation as sealed trait.

@hsaputra
Copy link
Contributor Author

If no more comment will merge this and 0.8.x branch by end of day tomorrow.

@StephanEwen
Copy link
Contributor

Looks good to me. It does not break the API, since it only moves source code, so I do not have any objections. I would merge it into the master, though, not into 0.8.

@hsaputra
Copy link
Contributor Author

The problem is that if we do not merge to 0.8.x branch then if we have fixes in those file would it create weird conflict when trying to cherry-pick?

@hsaputra
Copy link
Contributor Author

Can a brutha get some +1s for this one? =)

@StephanEwen
Copy link
Contributor

You have my +1

@hsaputra
Copy link
Contributor Author

Cool, thanks @StephanEwen, if no one beats me merging I will do this EOD today

@asfgit asfgit closed this in 7deeda7 Jan 23, 2015
@hsaputra hsaputra deleted the rename_coGroupDataSet_filename branch January 23, 2015 01:45
@hsaputra
Copy link
Contributor Author

As per recommendation from @StephanEwen, will NOT merge this to 0.8 until we need to cherry-pick fixes related to these files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants