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 style issues #2965

Merged
merged 3 commits into from
Jun 5, 2021
Merged

Fix style issues #2965

merged 3 commits into from
Jun 5, 2021

Conversation

rcurtin
Copy link
Member

@rcurtin rcurtin commented Jun 2, 2021

I noticed in #2964 that there are a bunch of style checks that are failing. I tried to fix them in this PR---let's see how successful I was. :)

@zoq
Copy link
Member

zoq commented Jun 3, 2021

@mlpack-jenkins test this please

@zoq
Copy link
Member

zoq commented Jun 3, 2021

I use the PR to check the cross-compilation job, hope that is fine.

@zoq
Copy link
Member

zoq commented Jun 3, 2021

@mlpack-jenkins test this please

@rcurtin
Copy link
Member Author

rcurtin commented Jun 3, 2021

Sounds good to me! 👍

@zoq
Copy link
Member

zoq commented Jun 3, 2021

@mlpack-jenkins test this please

@rcurtin rcurtin mentioned this pull request Jun 3, 2021
Closed
zoq
zoq approved these changes Jun 4, 2021
View reviewed changes
Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

Looks great to me.

@shrit
Copy link
Member

shrit commented Jun 4, 2021

@mlpack-jenkins test this please

@shrit
Copy link
Member

shrit commented Jun 4, 2021

Great, CrossCompilation check is working 👍

@shrit
Copy link
Member

shrit commented Jun 4, 2021

@rcurtin, this pull request seems to be cool, I am really interested to know what tool did you use in order to find all these style errors.
PS: I will try to give it a look tomorrow if it is not too late, I do not have any neurons left to handle it today.

@rcurtin
Copy link
Member Author

rcurtin commented Jun 4, 2021

I have no idea actually---Marcus set up the style checks job, and I just used the output from it and manually fixed everything. :)

@zoq
Copy link
Member

zoq commented Jun 4, 2021

We use cpplint for the style checks, you can find the script here - https://github.com/mlpack/jenkins-conf/tree/master/linter

mlpack-bot[bot]
mlpack-bot bot approved these changes Jun 5, 2021
View reviewed changes
Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

shrit
shrit approved these changes Jun 5, 2021
View reviewed changes
Copy link
Member

@shrit shrit left a comment

Choose a reason for hiding this comment

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

Great, pull request, thanks. 👍

@rcurtin rcurtin merged commit e33ff06 into mlpack:master Jun 5, 2021
@rcurtin rcurtin deleted the style-fixes branch June 5, 2021 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zoq zoq zoq approved these changes

@shrit shrit shrit approved these changes

@mlpack-bot mlpack-bot[bot] mlpack-bot approved these changes

Assignees
No one assigned
Labels
c: methods t: bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rcurtin @zoq @shrit