jerosoler / Drawflow

Simple flow library 🖥️🖱️
https://jerosoler.github.io/Drawflow/
MIT License
4.64k stars 721 forks source link

Warning and redundancy cleaning #202

Closed carmonium closed 3 years ago

carmonium commented 3 years ago

Hi,

first, thanks a lot you for the great job you've done.

Im testing to include DrawFlow in a project Im starting, and when included the source code, got about 8XX medium and minor warnings because of the use of var instead of let, some redundant code and stuff like that.

I cleaned 99% of that, but not changing the functionality of the original code. If it is useful for you in some way, let me know where should I send you the modified copy.

Regards, and thanks again.

LC

jerosoler commented 3 years ago

Hi @carmonium

Thank you.

I know the problem exists. I know a refactory is needed. If you want you can send a PR, although it will be difficult to review. Or send me an e-mail (In my profile it is available) with the code.

It is something that I have yet to do.

Jero

carmonium commented 3 years ago

Hi again,

attached you will find drawflow.js with the wanings cleaned. I guess it might save you a day of boring cleaning work.

​​​​​​I kept exactly the same order you disposed the functions, didn't refactor any of them, my goal was just to eliminate the warnings, so the modification could be briefed like this:

  1. Change every 'var ..." for 'let ...

  2. This led me to error when 'var' declaration was related to scoped variables, so moved those variables to the top of the scope, not top of the function, just of the scope to keep the structure.

  3. Commented out every not used function, but kept them it in the file.

  4. Commented out every parameter not used, but moved to the right in the same line as comment for you to see the missing paramenter.

  5. Deleted every commented piece of code, that I realized was already working in some other way (assumed the commented code was kept just in case).

Hope it helps. You have a really nice piece of code, let me know if I can help you some how, Im not that skilled in javascript as you are, but I can program well in Vue3(composition api) and python.

Why I mention python? Cause in my humble opinion you have a solution for the problem of monolitic approaches like NIFI o Streamsets, where processors and interface are all tied together, and all in Java.

With drawflow as it is, the interface could be agnostic ( for desing, and for flow monitoring) and server side (processors nodes) could be done in any language with the proto specification of your nodes.

Thanks again, and if you need an arquitecture counterpart, count on me.

Regards,

LC

El Jueves, Julio 08, 2021 02:30 -04, Jero Soler @.***> Ha escrito:     Hi @carmonium Thank you. I know the problem exists. I know a refactory is needed. If you want you can send a PR, although it will be difficult to review. Or send me an e-mail (In my profile it is available) with the code. It is something that I have yet to do. Jero — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

 

jerosoler commented 3 years ago

Hi @carmonium

Thanks, but.

In github it does not allow to attach files. Send in my profile e-mail: https://github.com/jerosoler

Jero

carmonium commented 3 years ago

Hi again,

couldn't pass it trough email, cause of security issues from your email server.

I putted in an S3 bucket, in this link https://tmp2020lc.s3.us-west-2.amazonaws.com/drawflow.js

Tell me please once you get it, to close the bucket.

Regards,

LC

El Viernes, Julio 09, 2021 07:17 -04, Jero Soler @.***> Ha escrito:     Hi @carmonium Thanks, but. In github it does not allow to attach files. Send in my profile e-mail: https://github.com/jerosoler Jero — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

 

jerosoler commented 3 years ago

Hi @carmonium

Thanks.

I already have it! You can now close the bucket.

I will make modifications when I fix things. Some like removing comments are already applied.

Jero