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 - A] Refactoring Kopia package #1546

Merged
merged 16 commits into from
Aug 2, 2022
Merged

[Kopia wrappers - A] Refactoring Kopia package #1546

merged 16 commits into from
Aug 2, 2022

Conversation

shlokc9
Copy link
Contributor

@shlokc9 shlokc9 commented Jul 7, 2022 •
edited

Change Overview

This PR consists of;

  1. refactoring changes in Kopia package
  2. command logsafe module
  3. utility methods in pkg/utils/utils.go

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

github-actions bot commented Jul 7, 2022

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 Jul 7, 2022
@shlokc9 shlokc9 changed the title (Kopia wrappers - A) Refactoring Kopia package [Kopia wrappers - A] Refactoring Kopia package Jul 7, 2022
@shlokc9 shlokc9 requested a review from pavannd1 July 11, 2022 10:02
PrasadG193
PrasadG193 reviewed Jul 11, 2022
View reviewed changes
pkg/kando/location_delete.go Outdated Show resolved Hide resolved
pkg/kopia/snapshot/snapshot.go Outdated Show resolved Hide resolved
@shlokc9 shlokc9 requested a review from PrasadG193 July 12, 2022 17:22
ankitjain235
ankitjain235 reviewed Jul 18, 2022
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

Changes look good overall.
We should start adding test coverage wherever we can.

pkg/kopia/utils.go Outdated Show resolved Hide resolved
pkg/kopia/utils.go Outdated Show resolved Hide resolved
pkg/stream/stream.go Outdated Show resolved Hide resolved
pavannd1
pavannd1 reviewed Jul 20, 2022
View reviewed changes
Copy link
Contributor

@pavannd1 pavannd1 left a comment •
edited

Choose a reason for hiding this comment

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

Looks good 🚀
You may have to move register.go into the repository package. That might help you fix the lint issues - not sure, just a guess

pkg/kopia/utils.go Show resolved Hide resolved
@shlokc9 shlokc9 force-pushed the kopia-wrappers-a branch 2 times, most recently from 118a6e5 to f419b1f Compare July 20, 2022 12:30
PrasadG193
PrasadG193 reviewed Jul 27, 2022
View reviewed changes
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

LGTM

ankitjain235
ankitjain235 reviewed Jul 27, 2022
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

LGTM.

pavannd1
pavannd1 approved these changes Aug 1, 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 Aug 1, 2022
@shlokc9 shlokc9 added the kueue label Aug 2, 2022
@mergify mergify bot merged commit 9198399 into master Aug 2, 2022
Kanister automation moved this from Reviewer approved to Done Aug 2, 2022
@mergify mergify bot deleted the kopia-wrappers-a branch August 2, 2022 03:59
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

@pavannd1 pavannd1 pavannd1 approved these changes

@viveksinghggits viveksinghggits Awaiting requested review from viveksinghggits

@ihcsim ihcsim Awaiting requested review from ihcsim

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@shlokc9 @pavannd1 @PrasadG193 @ankitjain235