Skip to content

Conversation

@MechCoder
Copy link
Contributor

No description provided.

@MechCoder
Copy link
Contributor Author

@mengxr

@SparkQA
Copy link

SparkQA commented Aug 19, 2015

Test build #41236 has finished for PR 8309 at commit d74ced6.

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

@mengxr
Copy link
Contributor

mengxr commented Aug 19, 2015

@MechCoder Shall we use @Since annotation directly?

@MechCoder
Copy link
Contributor Author

I thought we can merge all directly and replace everything at one go.

@mengxr
Copy link
Contributor

mengxr commented Aug 19, 2015

No, for existing PRs, we ask the authors to stay with @since tags because most of them are first-time contributors. For new PRs, we should use @Since annotation directly.

@MechCoder
Copy link
Contributor Author

Great. Will update in some time.

@MechCoder
Copy link
Contributor Author

@mengxr updated.

I wrote a small bash script that would do the conversion automatically, but there is one corner case that Ihad to take care of manually

@SparkQA
Copy link

SparkQA commented Aug 19, 2015

Test build #41278 has finished for PR 8309 at commit 3778da0.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • implicit class StringToColumn(val sc: StringContext)

@mengxr
Copy link
Contributor

mengxr commented Aug 19, 2015

@MechCoder Please also check the following:

  1. overridden methods that do not require JavaDoc (if the JavaDoc is the same as its parent)
  2. public variables in constructors (see examples in https://issues.apache.org/jira/browse/SPARK-9864)

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, we also need to add annotation to selectedFeatures.

@MechCoder
Copy link
Contributor Author

@mengxr I just updated the annotation for one class variable (numTopFeatures) ChiSqSelector but it does not show up in the scala doc for variables. (I have attached a screenshot)

screenshot from 2015-08-20 23 12 32

@SparkQA
Copy link

SparkQA commented Aug 20, 2015

Test build #41327 has finished for PR 8309 at commit 4abbcb3.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class ChiSqSelectorModel (
    • class ChiSqSelector (

@MechCoder
Copy link
Contributor Author

Actually the annotations on any class variable do not show up in the doc. Can you please verify?

@mengxr
Copy link
Contributor

mengxr commented Aug 20, 2015

Good catch! I will take a look.

@mengxr
Copy link
Contributor

mengxr commented Aug 20, 2015

Sent a patch #8344 to fix it.

@asfgit asfgit closed this in 7cfc075 Aug 20, 2015
asfgit pushed a commit that referenced this pull request Aug 20, 2015
Author: MechCoder <[email protected]>

Closes #8309 from MechCoder/tags_feature.

(cherry picked from commit 7cfc075)
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor

mengxr commented Aug 20, 2015

Merged into master and branch-1.5. Thanks! Please make a PR that convert all existing @SInCE tags into @SInCE annotation (but do not make additional changes to simplify the code review). Then we can have another PR to fix the missing ones.

@MechCoder MechCoder deleted the tags_feature branch August 21, 2015 03:45
@MechCoder
Copy link
Contributor Author

Thanks you for the patch.

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