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

Refactoring: Break out getRows #1169

Closed
knsv opened this issue Jan 2, 2020 · 2 comments · Fixed by #1268
Closed

Refactoring: Break out getRows #1169

knsv opened this issue Jan 2, 2020 · 2 comments · Fixed by #1268
Assignees
Labels
Area: Development Status: Approved Is ready to be worked on Type: Other Not an enhancement or a bug

Comments

@knsv
Copy link
Collaborator

knsv commented Jan 2, 2020

Code duplication
state/stateRenderer.js and state/shapes.js both contain identical implementations of getRows function.

It should be broken out to a common function. It is also similar to the sanitize function in flowDb. A more generic approach could cater all those use cases.

Create a folder called 'common' with common functions that are shared between the diagrams.

@knsv knsv added Type: Bug / Error Something isn't working or is incorrect Status: Triage Needs to be verified, categorized, etc Topic: Platform Type: Other Not an enhancement or a bug and removed Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Jan 2, 2020
@GDFaber
Copy link
Member

GDFaber commented Jan 2, 2020

I like the idea of centralizing functionality. Perhaps later on there will be more to put there, like splitting text lines with the line break regex (haven't double-checked that, just came to my mind when I was reading this).

@jgreywolf
Copy link
Contributor

We have a utility module, should we add things like this here, or not, as this would be common functionality more diagram specific?

@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: Development Status: Approved Is ready to be worked on Type: Other Not an enhancement or a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants