Badgeyay: Convert merge_badges.py into a class

[djmgit]

Presently os.system is used to execute merge_badges because merge_badges does have a proper interface/api to use it from another file. It is a standalone script. This was alright when badgeyay was a command line tool to create badges. However now, using os.system to run the script seems quite hacky and limits its use. Instead, merge_badges.py should be converted to a proper module containing a class
which can be imported to main.py and used. I guess this is the proper workflow rather than executing it as a stand alone script.

[djmgit]

@vaibhavsingh97 @gabru-md @ParthS007 your views?

[gabru-md]

Definitely,
I do understand that your point is very accurate. To put up simply, We are following a hackathon approach for badgeyay, by simply getting it worked.
We need to make it stable and do not rely on bash for executing the scripts. To design it into a full product we need to modify the files so that they can be compatible to each other and help in maintaining a proper workflow.

[djmgit]

@gabru-md yes

[ParthS007]

@djmgit Totally agree with you and we need to follow a modularized approach as you have proposed in the previous issue. :+1:

[yashLadha]

Why not create a Library and do changes into that and import that library into project. This approach i have seen in many org's that they create a package for their project and uses the higher level api's designed in the package. This way code will be modularized as well as easy to read and extendible. Views @djmgit @vaibhavsingh97 @gabru-md @ParthS007

Link to org projects Hydrus Scancode

[gabru-md]

Agreed !

[vaibhavsingh97]

Agreed 👍

On Tue 24 Apr, 2018, 5:27 PM Yash Ladha, [e-mail] wrote:

Why not create a Library and do changes into that and import that library
into project. This approach i have seen in many org's that they create a
package for their project and uses the higher level api's designed in the
package. This way code will be modularized as well as easy to read and
extendible. Views @djmgit https://github.com/djmgit @vaibhavsingh97
https://github.com/vaibhavsingh97 @gabru-md
https://github.com/gabru-md @ParthS007 https://github.com/ParthS007

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/fossasia/badgeyay/issues/628#issuecomment-383904143,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AITVan3BIhnyHI2T6NN9lzqlne8HRbcJks5trxMZgaJpZM4TVdWg
.

[ParthS007]

Agreed :+1:

[djmgit]

@yashLadha nice idea, yes we can definitely move merge_badges.py and generate_badges.py into a folder(perhaps badgeyay_utils or badgeyay_core) and convert it into a library. From the library we can import our required classes, methods etc. In that way, the core logic part will also remain separated from the web server part.

[djmgit]

Any updates?

[gabru-md]

I guess if no one is working on it at this moment I would like to claim this task then.

[yashLadha]

before proceeding @gabru-md we need to properly document this and layout the structure.

[yashLadha]

As already there is a cli present and we can extend the cli for the conversion of the class and the project. Views @gabru-md @djmgit

[gabru-md]

I can start with a new file and then maybe collaborate onto making it better ?
And once that is fully functioning and has proper docs we can replace it with the current file?

[yashLadha]

I would like to work on this @gabru-md . We can divide the work, If you want?

[gabru-md]

Okay!
I'll create a new file with proper usage of class and functions. And then we both can improve it. :+1:
Like making it better and adding docstrings

[djmgit]

@gabru-md @yashLadha great! @yashLadha yes you can surely extend the cli, just make sure nothing is breaking before making the replacement.

[yashLadha]

Ok So you proceed with converting the file into the Class and i will working on including the class into the library and docstrings. @gabru-md

[yashLadha]

Is this fine @gabru-md @djmgit ??

[gabru-md]

Sure ! I'll send a PR soon.
And meanwhile I'll discuss it with Yash on proceeding further