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

Merge others work #36

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Merge others work #36

wants to merge 3 commits into from

Conversation

edillmann
Copy link

I create this pull request to merge work done by apatil and calmh

Review on Reviewable

@mmlb
Copy link
Member

mmlb commented Feb 9, 2016

Hmm @edillmann I'll go ahead and merge @apatil's PR directly while taking care of the comments @trisk and I had. Also, unless he doesn't particularly want to and you need the functionality I'd rather @calmh submit the commits himself.

@mmlb
Copy link
Member

mmlb commented Feb 9, 2016

quick update: just ran into a failing test for Diff when testing #33 due to openzfs/zfs@141b638. Tests pass with a quick hack which needs correct handling.

@calmh
Copy link

calmh commented Feb 11, 2016

(The zfs package is something I used for one thing but currently don't, so I don't have the drive to integrate my changes, whatever they were, at the moment. :)

@mmlb
Copy link
Member

mmlb commented Feb 11, 2016

@calmh thats fair.

@edillmann can you rebase on top of master, I've merged w/ tweaks @apatil's PR.

@edillmann
Copy link
Author

I repushed this PR, tests are OK

@mmlb mmlb self-assigned this Feb 29, 2016
@mmlb
Copy link
Member

mmlb commented Mar 11, 2016

@edillmann breaking the API is a no-go. You're going to have to define other functions. I'd suggest something mimicking https://godoc.org/github.com/mistifyio/gozfs#Datasets, but unfortunately a different name :(


Review status: 0 of 4 files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants