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

[Kopia wrappers - B] Add command wrappers in the Kopia package #1526

Merged
merged 75 commits into from
Sep 7, 2022
Merged

[Kopia wrappers - B] Add command wrappers in the Kopia package #1526

merged 75 commits into from
Sep 7, 2022

Conversation

shlokc9
Copy link
Contributor

@shlokc9 shlokc9 commented Jun 30, 2022 •
edited

Change Overview

This PR creates wrappers to various kopia commands under kopia package

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • #XXX

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@github-actions
Copy link
Contributor

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Jun 30, 2022
pavannd1
pavannd1 reviewed Jun 30, 2022
View reviewed changes
pkg/apis/config/v1alpha1/const.go Outdated Show resolved Hide resolved
pkg/apis/config/v1alpha1/profile_types.go Outdated Show resolved Hide resolved
pkg/errors/errors.go Outdated Show resolved Hide resolved
pkg/kopia/profile.go Outdated Show resolved Hide resolved
pkg/kopia/utils.go Outdated Show resolved Hide resolved
pkg/kopia/kopia.go Outdated Show resolved Hide resolved
@shlokc9 shlokc9 changed the base branch from master to kopia-wrappers-a July 7, 2022 11:41
@shlokc9 shlokc9 changed the title Kopia wrappers (Kopia wrappers - B) Add command wrappers in the Kopia package Jul 7, 2022
@shlokc9 shlokc9 changed the title (Kopia wrappers - B) Add command wrappers in the Kopia package [Kopia wrappers - B] Add command wrappers in the Kopia package Jul 7, 2022
@shlokc9 shlokc9 marked this pull request as ready for review July 8, 2022 09:43
@shlokc9 shlokc9 requested a review from pavannd1 July 11, 2022 10:02
@shlokc9 shlokc9 requested a review from pavannd1 August 23, 2022 15:39
@shlokc9
Copy link
Contributor Author

shlokc9 commented Aug 23, 2022 •
edited

The golint issues are resolved in child PR #1557. Lint check will pass here once that PR is merged.

@ihcsim
Copy link
Contributor

ihcsim commented Aug 23, 2022

Personally, I prefer we review and merge this PR, only when there are actual code calling them, to avoid adding unused code. I have a feeling that some of these code will change, once we start adding the caller code. Or if we are sure that all these code will be used, and they have been reviewed and tested downstream, then why not just merge them now?

@pavannd1
Copy link
Contributor

pavannd1 commented Sep 1, 2022

I think you'd have to remove some unused consts to allow this PR to merge. You can add them in the following PR where they are actually used.

pavannd1
pavannd1 reviewed Sep 1, 2022
View reviewed changes
pkg/kopia/command/blob.go Outdated Show resolved Hide resolved
pavannd1
pavannd1 approved these changes Sep 3, 2022
View reviewed changes
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

LGTM 💯

Kanister automation moved this from In Progress to Reviewer approved Sep 3, 2022
@shlokc9 shlokc9 added the kueue label Sep 7, 2022
@mergify mergify bot merged commit 43fb264 into master Sep 7, 2022
Kanister automation moved this from Reviewer approved to Done Sep 7, 2022
@mergify mergify bot deleted the kopia-wrappers branch September 7, 2022 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PrasadG193 PrasadG193 PrasadG193 left review comments

@ankitjain235 ankitjain235 ankitjain235 left review comments

@viveksinghggits viveksinghggits Awaiting requested review from viveksinghggits

@ihcsim ihcsim Awaiting requested review from ihcsim

@pavannd1 pavannd1 Awaiting requested review from pavannd1

Assignees
No one assigned
Labels
kueue
Projects
Kanister
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shlokc9 @ihcsim @pavannd1 @PrasadG193 @ankitjain235