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

Refactor commands #5

Closed
wants to merge 1 commit into from
Closed

Conversation

@lex111
Copy link
Contributor

@lex111 lex111 commented Oct 4, 2020

Hi @andrewscaya, I hope you are well,

History: about a year ago, I expressed a desire to work on improving your project... And finally, I start doing refactoring, although maybe it doesn't make much sense, because it's just refactoring 😃

Anyway, this is only the first part of refactoring that I have planned. I want to split the codebase so that I end up making the commands easier to maintain and understand.

In this PR:

  • Introduce base class for commands BaseCommand to reduce code duplication (methods for Windows).
  • Use strict compassion everywhere
  • Rename variables to match camelCase variables convention
  • Minor improvements to naming methods and variables
  • Use array destructuring (this is controversial, but in the second step work I will think about it.)

Diff changes aren't big, so it shouldn't be hard to examine. Please take some time for them, I want to continue what I started.

@andrewscaya
Copy link
Contributor

@andrewscaya andrewscaya commented Oct 6, 2020

@lex111 Excellent! I'll certainly review your PR next week! Many thanks! :)

@andrewscaya
Copy link
Contributor

@andrewscaya andrewscaya commented Nov 11, 2020

Many thanks @lex111 ! I'll be merging your PR to branch 2.0.10-dev. Your changes will therefore be included in the next release of Linux for Composer!

@andrewscaya
Copy link
Contributor

@andrewscaya andrewscaya commented Nov 11, 2020

Merged on 2.0.10-dev, commit f9e8e2c.

@lex111
Copy link
Contributor Author

@lex111 lex111 commented Nov 11, 2020

@andrewscaya great, but I will continue refactoring in next PRs, as I wrote earlier, these are not final changes.

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.