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: fix function name in comments #18726

Merged
merged 1 commit into from
May 31, 2023
Merged

Fix: fix function name in comments #18726

merged 1 commit into from
May 31, 2023

Conversation

cuishuang
Copy link
Contributor

No description provided.

@cuishuang cuishuang requested a review from a team as a code owner May 24, 2023 01:58
OrlinVasilev
OrlinVasilev approved these changes May 25, 2023
View reviewed changes
Copy link
Member

@OrlinVasilev OrlinVasilev left a comment

Choose a reason for hiding this comment

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

lgtm

@OrlinVasilev OrlinVasilev added the release-note/update Update or Fix label May 25, 2023
@codecov
Copy link

codecov bot commented May 25, 2023 •
edited
Loading

Codecov Report

Merging #18726 (6ce1da9) into main (b021dbd) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head 6ce1da9 differs from pull request most recent head b3bf950. Consider uploading reports for the commit b3bf950 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18726      +/-   ##
==========================================
- Coverage   67.39%   67.37%   -0.02%     
==========================================
  Files         980      980              
  Lines      106750   106750              
  Branches     2665     2665              
==========================================
- Hits        71940    71927      -13     
- Misses      30945    30956      +11     
- Partials     3865     3867       +2
Flag Coverage Δ
unittests 67.37% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/common/http/transport.go 67.21% <ø> (ø)
src/lib/strings.go 100.00% <ø> (ø)
src/lib/trace/config.go 2.50% <ø> (ø)
src/pkg/reg/adapter/dockerhub/adapter.go 30.97% <ø> (ø)
src/server/middleware/repoproxy/proxy.go 2.70% <ø> (ø)
src/server/v2.0/handler/preheat.go 20.06% <ø> (ø)

... and 4 files with indirect coverage changes

@AllForNothing AllForNothing requested a review from MinerYang May 29, 2023 08:42
chlins
chlins approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins
Copy link
Member

chlins commented May 30, 2023

@cuishuang Hi, please rebase your branch with main.

@cuishuang
Copy link
Contributor Author

@cuishuang Hi, please rebase your branch with main.

Completed. Also do I need to squash multiple commits into one？

@MinerYang
Copy link
Contributor

Yes, please squash commit into one. Appreciate!
And we are wondering is there any specific linter you are using to test this?

@cuishuang
Copy link
Contributor Author

Yes, please squash commit into one. Appreciate! And we are wondering is there any specific linter you are using to test this?

Completed. I am very interested in static analysis, and I tried to write some analyzers for go vet, but the proposal was not approved.

Specifically, I did some analysis with the abstract syntax tree, but it is difficult to use it as a tool, because the first word in front of a large number of func is not necessarily the method name, so a lot of manual screening is required Work

@MinerYang
Copy link
Contributor

Hi @cuishuang please rebase your branch with main.

@cuishuang
Copy link
Contributor Author

Hi @cuishuang please rebase your branch with main.

Done. Thanks

@MinerYang MinerYang merged commit e19ec96 into goharbor:main May 31, 2023
@OrlinVasilev
Copy link
Member

@cuishuang congrats on merging your first PR here :)) Welcome to the club!

@cuishuang
Copy link
Contributor Author

@cuishuang congrats on merging your first PR here :)) Welcome to the club!

Thanks!

WilfredAlmeida pushed a commit to WilfredAlmeida/harbor that referenced this pull request Jul 8, 2023
@cuishuang @WilfredAlmeida
Fix: fix function name in comments (goharbor#18726)
1ae4b42 
fix function name in comments

Signed-off-by: cui fliter <[email protected]>
Signed-off-by: Wilfred Almeida <[email protected]>
WilfredAlmeida pushed a commit to WilfredAlmeida/harbor that referenced this pull request Jul 8, 2023
@cuishuang @WilfredAlmeida
Fix: fix function name in comments (goharbor#18726)
09fa6cd 
fix function name in comments

Signed-off-by: cui fliter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chlins chlins chlins approved these changes

@OrlinVasilev OrlinVasilev OrlinVasilev approved these changes

@zyyw zyyw zyyw approved these changes

@MinerYang MinerYang Awaiting requested review from MinerYang

Assignees

@MinerYang MinerYang

Labels
release-note/update Update or Fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cuishuang @chlins @MinerYang @OrlinVasilev @zyyw @stonezdj @AllForNothing