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

Add support for NextToLast shard function. #11

Merged
merged 2 commits into from Dec 16, 2016

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Dec 16, 2016

No description provided.

Also fix bug when only the Prefix function was being tested in most cases.
@kevina kevina added the status/in-progress In progress label Dec 16, 2016
@coveralls
Copy link

coveralls commented Dec 16, 2016

Coverage Status

Coverage increased (+0.8%) to 62.776% when pulling 4f600b596294122867eb4fe48e052c078f8055f3 on kevina/next-to-last into 9652294 on master.

@@ -30,6 +30,11 @@ func tempdir(t testing.TB) (path string, cleanup func()) {
return path, cleanup
}

func tryAllShardFuncs(t *testing.T, testFunc func(mkShardFunc, *testing.T)) {
t.Run("prefix", func(t *testing.T) { testFunc(flatfs.Prefix, t) })
t.Run("suffix", func(t *testing.T) { testFunc(flatfs.Suffix, t) })
Copy link
Member

Choose a reason for hiding this comment

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

Should we put the NextToLast shardfunc here too?

Copy link
Contributor Author

@kevina kevina Dec 16, 2016

Choose a reason for hiding this comment

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

Oops. Thanks for catching this. I tend to interactively commit changes to avoid commiting the rewritten paths and missed this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now.

@coveralls
Copy link

coveralls commented Dec 16, 2016

Coverage Status

Coverage increased (+0.8%) to 62.776% when pulling 0e6d035f405dbe45596915af3cabbc3311f6d64b on kevina/next-to-last into 9652294 on master.

@coveralls
Copy link

coveralls commented Dec 16, 2016

Coverage Status

Coverage increased (+0.8%) to 62.776% when pulling d4844ef on kevina/next-to-last into 9652294 on master.

Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks @kevina!

@whyrusleeping whyrusleeping merged commit 4965aa4 into master Dec 16, 2016
@whyrusleeping whyrusleeping deleted the kevina/next-to-last branch December 16, 2016 22:38
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Dec 16, 2016
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.

None yet

3 participants