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

Define a generalized function for display_submission_* #253

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

Define a generalized function for display_submission_* #253

wants to merge 4 commits into from

Conversation

hkmatsumoto
Copy link
Contributor

@hkmatsumoto hkmatsumoto commented Jan 11, 2020

Changes:

  • Define a generalized function for display_submission_*, since those functions will be very similar to each other.

@pushkalkatara
Copy link

@takitsuse I think what @vkartik97 meant was using a single submission_result util function to extract all useful content. Adding a new argument in the function i.e. display_submission_result(submission_id, file_name) where file_name can be stdout, stderr etc. Currently, we have all the implementation in utils and using it as a wrapper in the root directory for click library.

@hkmatsumoto
Copy link
Contributor Author

Okay, is @pushkalkatara's comment what you ment @vkartik97?

@krtkvrm
Copy link
Member

krtkvrm commented Jan 12, 2020

Okay, is @pushkalkatara's comment what you ment @vkartik97?

Yes 💯

@hkmatsumoto
Copy link
Contributor Author

Defined a generalized function as @pushkalkatara's proposal.
@vkartik97 Could you take another look?

@krtkvrm
Copy link
Member

krtkvrm commented Jan 12, 2020

Can you please fix the travis build?

@hkmatsumoto
Copy link
Contributor Author

@vkartik97 Fixed 👍

@hkmatsumoto
Copy link
Contributor Author

Should I merge this change to #252?

@hkmatsumoto hkmatsumoto changed the title Take in display_submission_result to result Define a generalized function for display_submission_* Jan 12, 2020
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.

3 participants