typed language

[AbdelRahmanSalah]

Your work is very nice but I highly recommend to refactor the code to typed language (that compile to Javascript) like Typescript or ELM, this will make the project maintainable, readable and extendable

[DrWaleedAYousef]

Currently, we are still working on forming the team. If you are interested in developing, pull requests are welcome.

[AbdelRahmanSalah]

yes I'm very interesting, I will refactor some code to typescript then send work in progress pull request to discuss the new design

[ndrwnaguib]

Dear @AbdelRahmanSalah, if you are still interested, would you please tell me when do you expect to submit your pull request? Also, you said:

I will refactor some code to typescript

Which parts of the code you are going to refactor it? So, we can work on the other parts.

[AbdelRahmanSalah]

Yes I'm still interested, and I'm currently going through the code to select the part which I can refactor to typescript, I will tell you shortly ISA the part i will refactor and the expected pull request date

[AbdelRahmanSalah]

Dear @andrewnaguib I will begin with Transform.js and I think it will take one week ISA according to my time

[ndrwnaguib]

Okay great, good luck and thanks for your willingness to contribute.

[AbdelRahmanSalah]

@andrewnaguib
I want to share with you some things I want to change and take your opinion:

• Functions should not depend on any global variables like lib.GoG_JSON[obj.data] all function requierment should be pass as function parameters so if I want to re-write the power function it should be like pow(obj: {power: number, field: string, name: string}, dataTable: Array<{[x: string]: any}>) this will make the power function maintainable and can be tested when we begin to write test cases

• we should try to avoid data mutation for data loaded from the source (like data loaded from csv )
  https://slemgrim.com/mutate-or-not-to-mutate/

• dynamic function call like results.push(Transform.prototype["R_" + i["function"]](i.properties)) very good but if user not send the function name right this line will throw exception and user will not know where is the error so I think we should handle function call manually by switch on the function name that sent from the user and throw meaning exception

[DrWaleedAYousef]

GoG is a library to be called by higher level graphical software to render to the browser. Therefore, before spending the time of converting to Typescript we need to make sure that this conversion will not throttle the rendering or cause calling limitation. The graphical software may call GoG to render hundreds of thousands of points or more.

[AbdelRahmanSalah]

Convert to typescript should not effect on performance or add any limitations.
my changes I recommend just general good practice and to help other contributors to understand the code flow

[AbdelRahmanSalah]

I recommend to begin use npm as a package manager because we should not push the third party libraries in the GOG github repository, I notice we use

• d3.js https://github.com/d3/d3
• jquery.js https://github.com/jquery/jquery
• datalib.js https://github.com/vega/datalib
• opencpu www.opencpu.org/js/archive/opencpu-0.4.js

[HishamElamir]

I think that we need in future to make GOG a standalone lib.

[AbdelRahmanSalah]

I think before adding any new feature we should refactor the repository to be suitable for contributors

[HishamElamir]

I know that, we should take in consideration the full documentation when we are working in code re-factor.

[DrWaleedAYousef]

This the first time I notice that (from your photo) you are Abdo Saleh, the ex-student at FCIH. يا أهلا وسهلا AbdelRahman Salah <[email protected]> writes:

…

I think before adding any new feature we should refactor the repository to be suitable for contributors — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.*

[AbdelRahmanSalah]

@DrWaleedAYousef I'm very happy you remembered me 😄
أهلا أهلا يادكتور

[AbdelRahmanSalah]

@HishamElamir could you create new branch by name refactoring for example so we can send pull requests on it before merge to master branch

and I have question Are you work with typescript before, cause I need to know the specification object type

[HishamElamir]

First things first, i think i do not have this type of permission that allows me to create branches.
And Second of all, yes i worked before with typescript, but i need to know what do you mean by

I need to know the specification object type

[AbdelRahmanSalah]

The specification object that used on HTML files and pass it to gog parser can you type it

[DrWaleedAYousef]

I will create the branch.

On Fri, Jul 27, 2018, 10:12 PM HishamElamir ***@***.***> wrote: First things first, i think i do not have this type of *permission* that allows me to create branches. And Second of all, yes i *worked* before with typescript, but i need to know what do you mean by I need to know the specification object type — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALpbzSdy2t05uBtUwoPPwA37dPqOtw6yks5uK3QwgaJpZM4VPyY3> .

@HishamElamir I tried creating the branch, I found that there is already a branch named "refactoring" created by you. However, I cannot see it on the web-interface!!! What is that?

[HishamElamir]

I checked the following @DrWaleedAYousef,

1. there's two branches master and 2015(created by yousef mohammed)
2. i do not have any branches in this repo

[DrWaleedAYousef]

Ok, I have created a new branch now called TestingRefactoring It is initialized with the same code at master. Please everyone check.

[AbdelRahmanSalah]

@HishamElamir First PR submitted please review I create folder tsVersion then we can begin convert each lib to typescript in this folder then we can remove all js files

[DrWaleedAYousef]

Were you able to push to the new branch?

…

On Sat, Aug 11, 2018, 10:49 AM AbdelRahman Salah ***@***.***> wrote: @HishamElamir <https://github.com/HishamElamir> First PR please review I create folder tsVersion then we can begin convert each lib to typescript in this folder then we can remove all js files — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALpbzS7dRmWrFHhwoP5oN8Y_wp0JKAA2ks5uPpqUgaJpZM4VPyY3> .

[AbdelRahmanSalah]

I don't think I have permission to push directly to the repository
And I recommend three steps before merging code on hci-lab/gog
1- each contributor should fork from the main repo to his own repo
2- after contributor make changes in his repo, he should send Pull request to hci-lab/gog refactoring branch
3- his changes reviewed from the main contributors in the repository if they approved the changes, one of them can merge the code

[DrWaleedAYousef]

Sure; this is the plan. This is why we created the branch.

…

On Sat, Aug 11, 2018, 4:30 PM AbdelRahman Salah ***@***.***> wrote: I don't think I have permission to push directly to the repository And I recommend three steps before merging code on hci-lab/gog 1- each contributor should fork from the main repo to his own repo 2- after contributor make changes in his repo, he should send Pull request to hci-lab/gog refactoring branch 3- his changes reviewed from the main contributors in the repository if they approved the changes, one of them can merge the code — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALpbzVhUF67ZPSLYwFSXkibxAIOM9Pc5ks5uPupvgaJpZM4VPyY3> .