search

makinacorpus / DbToolsBundle

A PHP library to backup, restore and anonymize databases
https://dbtoolsbundle.readthedocs.io
MIT License
181 stars 15 forks source link

No issue - Refactor code related to backuppers and restorers #100

Closed Lonnytunes closed 8 months ago

Lonnytunes commented 9 months ago

@pounard @SimonMellerin

I created the directory src/Utility in a previous PR for the CommandLine class.

I added AbstractProcessTrait (to rename?) and ChainLogger elements into this directory for now.

But I think CommandLine and AbstractProcessTrait could be in an src/Process directory, and ChainLogger in an src/Log.

Let me know your opinions! :slightly_smiling_face:

pounard commented 9 months ago

@Lonnytunes

But I think CommandLine and AbstractProcessTrait could be in an src/Process directory, and ChainLogger in an src/Log.

I have no strong opinion about this. For the first part, the Process namespace is a good idea, for the other one, I think that the Utility namespace must be removed, and the ChainLogger moved into the Helper namespace, which is basically the same.

Let's keep only one "fourzytou" namespace (sorry, french joke, but you get what I mean).

I'do go further and place the Process namespace under Helper, such as this: src/Helper/Process/....

Lonnytunes commented 9 months ago

Thanks for your answer @pounard

I will move ChainLogger from the Utility namespace to the Helper one. It's actually a kind of duplicate.

But I prefer to keep the idea of the Process namespace at the root for now.

Are you agree to rename AbstractProcessTrait to ProcessTrait?

pounard commented 8 months ago

Are you agree to rename AbstractProcessTrait to ProcessTrait?

Yes, it's either abstract, either a trait, but not both.