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.

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

Fix comments style to pass scala style check #461

Merged
merged 3 commits into from
Jul 30, 2020
Merged

Fix comments style to pass scala style check #461

merged 3 commits into from
Jul 30, 2020

Conversation

LuciferYang
Copy link
Contributor

There are 4 files in shuffle-plugin module use Incorrect comments format as following:

  • UCXTransaction.scala
  • UCXShuffleTransport.scala
  • UCX.scala
  • UCXConnection.scala

They don't follow the NoScalaDoc check rule, this pr fix these to pass scala style check.

@svcngcc
Copy link
Contributor

svcngcc commented Jul 29, 2020

Can one of the admins verify this patch?

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jul 29, 2020 •
edited
Loading

gentle ping @jlowe @tgravescs, please have a review, thx

tgravescs
tgravescs previously approved these changes Jul 29, 2020
View reviewed changes
@tgravescs
Copy link
Collaborator

build

jlowe
jlowe previously approved these changes Jul 29, 2020
View reviewed changes
@revans2
Copy link
Collaborator

revans2 commented Jul 29, 2020

Thanks for the fix this is great, but legal requires that you sign your commits as described here. We don't have CI setup yet to enforce this but it should be coming soon.

https://github.com/NVIDIA/spark-rapids/blob/branch-0.2/CONTRIBUTING.md#sign-your-work

revans2
revans2 requested changes Jul 29, 2020
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

Need to have commits signed

andygrove
andygrove previously approved these changes Jul 29, 2020
View reviewed changes
@jlowe
Copy link
Member

jlowe commented Jul 29, 2020

Need to have commits signed

To be clear, we need the commits to be signed-off rather than signed. We do not require that commits are cryptographically signed. The sign-off essentially boils down to adding the -s flag when you perform the git commit as a conscious decision that you agree to the Developer Certificate of Origin.

This was referenced Jul 29, 2020
Merged
Merged
@LuciferYang LuciferYang dismissed stale reviews from andygrove, jlowe, and tgravescs via a038032 July 30, 2020 02:20
@LuciferYang
revert empty line
90e252d 
Signed-off-by: yangjie01 <[email protected]>
@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jul 30, 2020 •
edited
Loading

Thank you for your review @jlowe @revans2 @tgravescs , is ok now or "commit -s " required for the first commit?

@revans2
Copy link
Collaborator

revans2 commented Jul 30, 2020

build

revans2
revans2 approved these changes Jul 30, 2020
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

I think technically legal wants it for all commits, but in this case I am happy to merge it in because it should be clear for this PR the intention

@LuciferYang
Copy link
Contributor Author

@revans2 Thank you for your explanation ~

@abellina abellina self-requested a review July 30, 2020 13:42
@revans2
Copy link
Collaborator

revans2 commented Jul 30, 2020

The test failure from CI appeared to be a flaky test, so I am going to merge this anyways.

@revans2 revans2 merged commit d7935c1 into NVIDIA:branch-0.2 Jul 30, 2020
@jlowe
Copy link
Member

jlowe commented Jul 30, 2020

I filed #473 to track the test failure.

@jlowe jlowe mentioned this pull request Jul 30, 2020
Merged
@LuciferYang LuciferYang deleted the fix-comments-style branch July 30, 2020 15:26
@sameerz sameerz added the build Related to CI / CD or cleanly building label Jul 31, 2020
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@LuciferYang
Fix comments style to pass scala style check (NVIDIA#461)
caa0a81 
* fix comments style

* add a empty line to commit signoff

Signed-off-by: yangjie01 <[email protected]>

* revert empty line

Signed-off-by: yangjie01 <[email protected]>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@LuciferYang
Fix comments style to pass scala style check (NVIDIA#461)
53372fa 
* fix comments style

* add a empty line to commit signoff

Signed-off-by: yangjie01 <[email protected]>

* revert empty line

Signed-off-by: yangjie01 <[email protected]>
pxLi pushed a commit to pxLi/spark-rapids that referenced this pull request May 12, 2022
@yhwen
Fixed the missing min_sites and required_sites for Job. (NVIDIA#461)
eaf1b5b 
* Fixed the missing min_sites and required_sites for Job.

* Removed the duplicate constant.

* fixed codestyle.

* fixed typo, also enhanced default value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revans2 revans2 revans2 approved these changes

@abellina abellina abellina approved these changes

@tgravescs tgravescs tgravescs left review comments

@jlowe jlowe jlowe left review comments

@andygrove andygrove andygrove left review comments

Assignees
No one assigned
Labels
build Related to CI / CD or cleanly building
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@LuciferYang @svcngcc @tgravescs @revans2 @jlowe @andygrove @abellina @sameerz