Skip to content

Fixing the formatting directives error during compilation#2420

Merged
mlaventure merged 1 commit intocontainerd:masterfrom
sudeeshjohn:master
Jul 5, 2018
Merged

Fixing the formatting directives error during compilation#2420
mlaventure merged 1 commit intocontainerd:masterfrom
sudeeshjohn:master

Conversation

@sudeeshjohn
Copy link
Copy Markdown
Contributor

@sudeeshjohn sudeeshjohn commented Jun 26, 2018 •
edited
Loading

Signed-off-by: Sudeesh John sudeesh@linux.vnet.ibm.com

Ref: #2404

estesp
estesp reviewed Jun 26, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

thanks for the PR; comment on the logrus.Info change

cmd/containerd-release/main.go Outdated
Copy link
Copy Markdown
Member

@estesp estesp Jun 26, 2018

Choose a reason for hiding this comment

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

any reason to not just change Info to Infof? having the project name prior to release tool... will be clearer IMO

Copy link
Copy Markdown
Contributor Author

@sudeeshjohn sudeeshjohn Jun 27, 2018

Choose a reason for hiding this comment

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

@estesp agree . Will resend the patch . thanks for the review.

@estesp
Copy link
Copy Markdown
Member

estesp commented Jun 26, 2018

Also, your Signed-off-by: Sudeesh John sudeesh@linux.vnet.ibm.com needs to be in the commit message, not just a PR comment for the DCO check to pass on CI

@sudeeshjohn sudeeshjohn force-pushed the master branch 7 times, most recently from 0397879 to 412bc7b Compare June 27, 2018 15:45
@sudeeshjohn
Copy link
Copy Markdown
Contributor Author

sudeeshjohn commented Jul 3, 2018

@estesp can you please check the newly sent patch

cpuguy83
cpuguy83 approved these changes Jul 3, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

estesp
estesp approved these changes Jul 4, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

thaJeztah
thaJeztah approved these changes Jul 4, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM (not a maintainer)

@thaJeztah thaJeztah mentioned this pull request Jul 5, 2018
Merged
@mlaventure
Copy link
Copy Markdown
Contributor

mlaventure commented Jul 5, 2018

just a nit, could you change the commit message to Fix the formatting directives... ? (i.e. replacing Fixing with Fix)

@sudeeshjohn
Fix the formatting directives error during compilation
5a4f007 
Signed-off-by: Sudeesh John <sudeesh@linux.vnet.ibm.com>
@sudeeshjohn
Copy link
Copy Markdown
Contributor Author

sudeeshjohn commented Jul 5, 2018

@mlaventure done

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 5, 2018

Codecov Report

Merging #2420 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2420   +/-   ##
======================================
  Coverage      45%     45%           
======================================
  Files          92      92           
  Lines        9412    9412           
======================================
  Hits         4236    4236           
  Misses       4493    4493           
  Partials      683     683
Flag Coverage Δ
#linux 49.23% <ø> (ø) ⬆️
#windows 41.3% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b0b412...5a4f007. Read the comment docs.

@mlaventure
Copy link
Copy Markdown
Contributor

mlaventure commented Jul 5, 2018

Thanks!

@mlaventure mlaventure merged commit 5900361 into containerd:master Jul 5, 2018
@thaJeztah thaJeztah mentioned this pull request Mar 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@estesp estesp estesp approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sudeeshjohn @estesp @mlaventure @codecov-io @cpuguy83 @thaJeztah