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: Scripts converted into classes #30

Open
flachica opened this issue Aug 16, 2020 · 3 comments
Open

Refactoring: Scripts converted into classes #30

flachica opened this issue Aug 16, 2020 · 3 comments

Comments

@flachica
Copy link
Contributor

In this PR I propossed the refactoring to

* Module script is converted to instance of class
* It is easy to convert, only need move the current content of script to __init__ class
* All script have the run base function that operates with protected properties _TEXT_REPLACES, ERRORS, ETC. `def run(self, module_path, manifest_path, module_name, migration_steps, directory_path, commit_enabled)`
* Also have `def process_file(self, root, filename, extension, file_renames, directory_path, commit_enabled)` to process file one by one
* If a script fail you can know so easy. Only need review the stack trace.
* On this refactoring you could delete global functions. Only you need inherit run method and overwrite it
* I have maintain global_funtions

Since I intend to continue working on the tool and I prefer that the author give me his opinion, I am going to keep a branch in my fork and I will continue doing PR in the master of this repository. I await the code review. If they give me OK, when the time comes I will do a PR with the refactoring

@flachica
Copy link
Contributor Author

flachica commented Oct 8, 2020

Can someone review this refactoring? On my fork, in the flachica_features branch, I made some changes that I think may be interesting.

Refactoring is totally optional but I found it useful. Also, on my branch I included those commits that the author could not review due to work overload

@ivantodorovich

@ivantodorovich
Copy link
Collaborator

Hi @flachica ,

Sure, that PR is closed though. Can you reopen it and possibly rebase it?

@flachica
Copy link
Contributor Author

flachica commented Oct 8, 2020

Many thanks. PR Opened, If you see it clearly, you could clean the history once the work is validated. @ivantodorovich

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants