Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upEnvironment table #286
Environment table #286
Conversation
|
Great start! Please see comments. :) |
| @@ -336,6 +336,38 @@ def _append_video(self, extra, extra_index, test_index): | |||
| ) | |||
| self.additional_html.append(html.div(html_div, class_="video")) | |||
|
|
|||
| class EnvironmentTable: | |||
| def __init__(self, config): | |||
This comment has been minimized.
This comment has been minimized.
BeyondEvil
Mar 20, 2020
Contributor
I would split this big init up into smaller functions.
Think builder pattern.
This comment has been minimized.
This comment has been minimized.
werdeil
Mar 20, 2020
Author
Contributor
Not really sure to understand what to do here, I agree that everything in the init is not good but the function I can think of (generate_header, generate_row, ...) are very small... Do you want something like that?
This comment has been minimized.
This comment has been minimized.
BeyondEvil
Mar 20, 2020
Contributor
Yeah, I'm having trouble articulating the vision I have in my head. Give me the weekend to see if I can throw something together that you can refine. :)
|
Regarding builder pattern. We can have a default configuration and then users can use hooks to add to that config. |
|
Hi @BeyondEvil I just pushed something, tell me if it is what you are thinking when you say "builder pattern" |
|
How about a rebase? |
|
@ssbarnea Rebase done but there are failing tests, I guess they are new, I'll have a look |
werdeil commentedMar 20, 2020
Following remarks made on PR 281, here is a first version of the EnvironmentTable class which allow to use new hooks.
@BeyondEvil tell me if it is what you had in mind.