Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

Adjust the word order of comments #947

Merged
merged 2 commits into from
Sep 23, 2019

Conversation

yuxiaobo96
Copy link
Contributor

Signed-off-by: yuxiaobo [email protected]

Ⅰ. Describe what this PR did

Adjust the word order of comments.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Don't need.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Sep 20, 2019

Codecov Report

Merging #947 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #947      +/-   ##
==========================================
+ Coverage   46.45%   46.47%   +0.01%     
==========================================
  Files         114      114              
  Lines        6737     6737              
==========================================
+ Hits         3130     3131       +1     
  Misses       3351     3351              
+ Partials      256      255       -1
Impacted Files Coverage Δ
dfdaemon/config/config.go 87.6% <ø> (ø) ⬆️
cmd/supernode/app/options.go 100% <ø> (ø) ⬆️
dfdaemon/proxy/proxy.go 8.54% <ø> (ø) ⬆️
supernode/daemon/mgr/scheduler/manager.go 22.6% <0%> (+0.68%) ⬆️

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 a37632b...4186e93. Read the comment docs.

@@ -364,7 +364,7 @@ func certPoolFromFiles(files ...string) (*x509.CertPool, error) {
return roots, nil
}

// Proxy describe a regular expression matching rule for how to proxy a request
// Proxy describes a regular expression matching rule for how to proxy a request
Copy link
Member

Choose a reason for hiding this comment

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

I think there should be a dot at the end of comment // Proxy describes a regular expression matching rule for how to proxy a request -> // Proxy describes a regular expression matching rule for how to proxy a request . ,it will be better? the other comment are the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the same as you think, but there are too many comments that don't have the dot.
I am going to add the dot together for these comments in the next pr.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's OK to fix this comment in this PR. THX. 😄

Copy link
Member

@SataQiu SataQiu left a comment

Choose a reason for hiding this comment

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

Thanks @yuxiaobo96
/lgtm
/assign @starnop

@@ -188,7 +188,7 @@ func (proxy *Proxy) mirrorRegistry(w http.ResponseWriter, r *http.Request) {
}

// remoteConfig returns the tls.Config used to connect to the given remote host.
// If the host should not be hijacked, nil will be returned.
// If the host should not be hijacked, it will return nil.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a conjunction after the comma.

@@ -364,7 +364,7 @@ func certPoolFromFiles(files ...string) (*x509.CertPool, error) {
return roots, nil
}

// Proxy describe a regular expression matching rule for how to proxy a request
// Proxy describes a regular expression matching rule for how to proxy a request
Copy link
Contributor

Choose a reason for hiding this comment

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

It's OK to fix this comment in this PR. THX. 😄

Signed-off-by: yuxiaobo <[email protected]>
@yuxiaobo96
Copy link
Contributor Author

I have already modified in the pr.

@yuxiaobo96
Copy link
Contributor Author

/assign @starnop

@starnop
Copy link
Contributor

starnop commented Sep 23, 2019

LGTM.

@starnop starnop merged commit d200985 into dragonflyoss:master Sep 23, 2019
starnop added a commit to starnop/Dragonfly that referenced this pull request Nov 27, 2019
inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants