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

[WIP] Added file system level collaboration #821

Closed
wants to merge 22 commits into from

Conversation

hkirat
Copy link

@hkirat hkirat commented Jul 4, 2017

This PR implements the next step for collaboration, keeping file systems in sync.

@gideonthomas
Copy link

@hkirat can you rebase this branch as well?

@hkirat
Copy link
Author

hkirat commented Jul 6, 2017

I think this is up to date with collaboration @gideonthomas
Could you confirm?

@hkirat
Copy link
Author

hkirat commented Jul 6, 2017

I think the reason the previous commits are showing up is because the previous PR was squashed.

@hkirat
Copy link
Author

hkirat commented Jul 7, 2017

Implemented comments mentioned here:
https://github.com/mozilla/thimble.mozilla.org/issues/2351

@gideonthomas
Copy link

@hkirat, this PR is a bit of a mess at the moment. Can you clean this up? My suggestion is to close this PR and open new ones. Keep in mind that each PR (and thus each branch) should only address 1 issue ideally (this PR seems to be fixing multiple issues making it really hard to read). Don't hesitate to open multiple PRs. Whenever you see that you're out of date with the branch you're merging into, always rebase, don't merge. If you need to (due to squashing of commits, or something else), use git rebase --onto (https://content.pivotal.io/blog/git-rebase-onto is a good description of it), so that the PR's commits always stay at the top. Without that it becomes really hard to review.

@hkirat
Copy link
Author

hkirat commented Jul 10, 2017

Closing for now.
Will work on file system collaboration after a little restructuring in the current collaboration branch

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.

2 participants