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

Seperate out write to file #1127

Merged
merged 4 commits into from May 22, 2017
Merged

Seperate out write to file #1127

merged 4 commits into from May 22, 2017

Conversation

fredkingham
Copy link
Contributor

No description provided.

renderer.write_to_file(full_file_name)
z.write(full_file_name, zip_relative_file_path(file_name))
generate_files(os.path.join(target_dir, zipfolder), episodes, user)
file_names = generate_files(root_dir, episodes, user)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand why we're calling generate_files twice?

@fredkingham
Copy link
Contributor Author

@davidmiller great call, done!

@@ -252,6 +252,42 @@ def __init__(self, model, episode_queryset, user, fields=None):
)


def generate_files(root_dir, episodes, user):
""" Generate the files and return a tuple of absolute_file_name, file_name
Copy link
Member

Choose a reason for hiding this comment

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

Referring to the files makes me think we're in a spy drama ;)

Can we call these e.g. generate_full_extract_files for more spelling but less likelihood of renaming later ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you say full extract because this will be in contrast to having inline extracts?

@fredkingham
Copy link
Contributor Author

@davidmiller what do you think of generate_csv_files?

@davidmiller davidmiller merged commit c78d367 into v0.8.2 May 22, 2017
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

2 participants