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.

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

Kopia Integration: Unified Repository Provider - Implementation #5179

Merged
merged 1 commit into from
Aug 16, 2022

Conversation

Lyndon-Li
Copy link
Contributor

@Lyndon-Li Lyndon-Li commented Aug 4, 2022

Add changes for Kopia Integration: Unified Repository Provider - method implementation
Related Issue: ##5079

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2022

Codecov Report

Merging #5179 (5d61e62) into main (088eb9b) will increase coverage by 0.18%.
The diff coverage is 48.71%.

@@            Coverage Diff             @@
##             main    #5179      +/-   ##
==========================================
+ Coverage   41.32%   41.50%   +0.18%     
==========================================
  Files         214      214              
  Lines       18710    18816     +106     
==========================================
+ Hits         7731     7809      +78     
- Misses      10405    10431      +26     
- Partials      574      576       +2     
Impacted Files Coverage Δ
pkg/repository/provider/unified_repo.go 60.26% <48.71%> (+7.38%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Lyndon-Li Lyndon-Li force-pushed the udmrepo-dev-02 branch 4 times, most recently from e97f403 to 1045661 Compare August 5, 2022 10:10
pkg/repository/udmrepo/service/service.go Outdated Show resolved Hide resolved
pkg/repository/provider/unified_repo.go Outdated Show resolved Hide resolved
pkg/repository/provider/unified_repo.go Outdated Show resolved Hide resolved
pkg/repository/provider/unified_repo.go Outdated Show resolved Hide resolved
@Lyndon-Li Lyndon-Li force-pushed the udmrepo-dev-02 branch 3 times, most recently from 5871bba to 53fc3ad Compare August 9, 2022 08:19
reasonerjt
reasonerjt previously approved these changes Aug 9, 2022
pkg/repository/provider/unified_repo.go Show resolved Hide resolved

func GetRepoUser() (username, domain string) {
return defaultUsername, defaultDomain
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In one of my functions that needs to get Repo User info, so I must need to import service package, but nothing to do with udmrepo service, should it be one configuration for udmrepo rather than configuration in service?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the discussion, the service package means the package to provide all the methods exposed by the unified repo. In this way, it is rational to put the GetRepoUser in the service package.

However, the package name may be misleading that the package binds to the BackupRepoService interface. Therefore:

  • Let's add some comments to clarify the usage of the package
  • When we see a better name in future, we will change the package name

@qiuming-best qiuming-best merged commit 71e4430 into vmware-tanzu:main Aug 16, 2022
@Lyndon-Li Lyndon-Li deleted the udmrepo-dev-02 branch August 16, 2022 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants