Conversation
BeyondEvil
left a comment
BeyondEvil
left a comment
There was a problem hiding this comment.
Great start!
Please see comments. :)
| self.additional_html.append(html.div(html_div, class_="video")) | ||
|
|
||
| class EnvironmentTable: | ||
| def __init__(self, config): |
There was a problem hiding this comment.
I would split this big init up into smaller functions.
Think builder pattern.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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? |
werdeil
force-pushed
the
environment_table
branch
from
a064b92 to
8e14277
Compare
|
@ssbarnea Rebase done but there are failing tests, I guess they are new, I'll have a look |
|
I'd like to see a revamp of this for v4 if @werdeil is up for it. |
3 participants
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.