Skip to content

Add initial version#1

Merged
insanoid merged 3 commits intomasterfrom
initial-version
Oct 16, 2017
Merged

Add initial version#1
insanoid merged 3 commits intomasterfrom
initial-version

Conversation

@jaysonsantos
Copy link
Copy Markdown
Contributor

No description provided.

@jaysonsantos jaysonsantos force-pushed the initial-version branch 3 times, most recently from 3737448 to 6bfca88 Compare October 13, 2017 15:32
Signed-off-by: Jayson Reis <[email protected]>
src/efs/efs3.py Outdated
@@ -0,0 +1,58 @@
"""Eatfirst FileSystem for S3."""
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💄 "EatFirst", capital "F".

There are other wrong comments with this spelling.

src/efs/efs3.py Outdated
from fs.s3fs import thread_local


class EFS3(S3FS):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Now that we are messing with this untouched part of the code... why not EatFirstS3?

from .efs3 import EFS3


class EFS:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Now that we are messing with this untouched part of the code... why not a better name like EatFirstFS or EatFirstFileSystem?

EFS and EFS3 look very similar, and they should not be.
EFS3 looks like the version 3 of EFS.

docs/usage.rst Outdated

import efs

# There is no need to initialise it because it will always the current app.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💄 A verb is missing in the sentence, right? "It will always [?] the current app"

Signed-off-by: Jayson Reis <[email protected]>
@insanoid insanoid merged commit 8509917 into master Oct 16, 2017
@insanoid insanoid deleted the initial-version branch October 16, 2017 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants