Skip to content

SPARK-8916 [Documentation, MLlib] Add @since tags to mllib.regression #7518

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.

Sign up for GitHub

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

Jump to bottom
Closed
prayagchandran wants to merge 4 commits into from
Closed

SPARK-8916 [Documentation, MLlib] Add @since tags to mllib.regression #7518

prayagchandran wants to merge 4 commits into from

Conversation

@prayagchandran
Copy link
Contributor

@prayagchandran prayagchandran commented Jul 19, 2015

Added @SInCE tags to mllib.regression

JoshRosen
JoshRosen reviewed Jul 19, 2015
View reviewed changes
mllib/src/main/scala/org/apache/spark/mllib/regression/StreamingLinearAlgorithm.scala Outdated
Copy link
Contributor

@JoshRosen JoshRosen Jul 19, 2015

Choose a reason for hiding this comment

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

This is mis-formatted; the line with the /** shouldn't have the method description on it (see the other multi-line Scaladoc strings in this file for examples of the proper style).

Copy link
Contributor Author

@prayagchandran prayagchandran Jul 19, 2015

Choose a reason for hiding this comment

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

I see. I will correct this to match other Scaladoc strings.

@prayagchandran
Copy link
Contributor Author

prayagchandran commented Jul 19, 2015

@JoshRosen I reformatted the comments now. Also added a few more tags I had missed. Please take a look.

@JoshRosen
Copy link
Contributor

JoshRosen commented Jul 20, 2015

Formatting seems fine to me, but I'll let @mengxr or @jkbradley do the final sign-off on this (I'm not really a MLlib reviewer).

Jenkins, this is ok to test.

@prayagchandran
Copy link
Contributor Author

prayagchandran commented Jul 20, 2015

Thanks Josh!

@mengxr
Copy link
Contributor

mengxr commented Jul 21, 2015

cc @dbtsai

@mengxr
Copy link
Contributor

mengxr commented Jul 21, 2015

ok to test

@SparkQA
Copy link

SparkQA commented Jul 21, 2015

Test build #37875 has finished for PR 7518 at commit 1a0365f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MechCoder
Copy link
Contributor

MechCoder commented Aug 13, 2015

In general: we need not add tags to equals, hashCode, toString and private / protected methods. Please make those changes

MechCoder
MechCoder reviewed Aug 13, 2015
View reviewed changes
mllib/src/main/scala/org/apache/spark/mllib/regression/GeneralizedLinearAlgorithm.scala Outdated
Copy link
Contributor

@MechCoder MechCoder Aug 13, 2015

Choose a reason for hiding this comment

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

How were you able to tag this? Any version less that 1.0.0 gives me fatal: Invalid object name error.

Copy link
Contributor Author

@prayagchandran prayagchandran Aug 14, 2015

Choose a reason for hiding this comment

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

I hadn't checked for the right signature of the method. I will correct this and add other updates to the PR soon.

@jkbradley
Copy link
Member

jkbradley commented Aug 13, 2015

(updated) Ignore my old comment. You can keep since tags. We'll switch to annotations later.

@jkbradley
Copy link
Member

jkbradley commented Aug 13, 2015

@prayagchandran It will be great to merge this ASAP to include it in the 1.5 RC. I'll watch for updates. Thanks!

dbtsai
dbtsai reviewed Aug 14, 2015
View reviewed changes
mllib/src/main/scala/org/apache/spark/mllib/regression/IsotonicRegression.scala Outdated
Copy link
Member

@dbtsai dbtsai Aug 14, 2015

Choose a reason for hiding this comment

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

Should be /**

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #40906 has finished for PR 7518 at commit 6c6d584.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mengxr
Copy link
Contributor

mengxr commented Aug 17, 2015

@dbtsai Do you have time to make a final pass?

asfgit pushed a commit that referenced this pull request Aug 18, 2015
@prayagchandran
SPARK-8916 [Documentation, MLlib] Add @SInCE tags to mllib.regression
f5ed9ed 
Added since tags to mllib.regression

Author: Prayag Chandran <[email protected]>

Closes #7518 from prayagchandran/sinceTags and squashes the following commits:

fa4dda2 [Prayag Chandran] Re-formatting
6c6d584 [Prayag Chandran] Corrected a few tags. Removed few unnecessary tags
1a0365f [Prayag Chandran] Reformating and adding a few more tags
89fdb66 [Prayag Chandran] SPARK-8916 [Documentation, MLlib] Add @SInCE tags to mllib.regression

(cherry picked from commit 18523c1)
Signed-off-by: DB Tsai <[email protected]>
@asfgit asfgit closed this in 18523c1 Aug 18, 2015
@dbtsai
Copy link
Member

dbtsai commented Aug 18, 2015

Thanks. Merged into master and 1.5 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@prayagchandran @JoshRosen @mengxr @SparkQA @MechCoder @jkbradley @dbtsai