aimeos / ai-controller-jobs

Aimeos e-commerce job controllers
https://aimeos.org
GNU Lesser General Public License v3.0
28 stars 17 forks source link

Add Supplier Processor to CSV Import #25

Closed alextravin closed 4 years ago

alextravin commented 4 years ago

Please note I've add ability to create a new one Supplier at \Aimeos\Controller\Common\Product\Import\Csv\Cache\Supplier\Standard::get() if existing one is not founded. Is it okay?

aimeos commented 4 years ago

Thank you for your contribution! Some comments have been added where necessary.

Regarding automatic creation of suppliers: This may not be a good idea because it can lead to a lot of garbage if wrong supplier codes are used in the CSV. We want to create a separate supplier CSV import instead.

alextravin commented 4 years ago

a lot of garbage if wrong supplier codes are used

Now I see, will remove it

We want to create a separate supplier CSV import instead.

Can I do that by creating a new structure at folder ai-controller-jobs/controller/common/src/Controller/Common/Supplier ? I want to.

aimeos commented 4 years ago

Can I do that by creating a new structure at folder ai-controller-jobs/controller/common/src/Controller/Common/Supplier ? I want to.

There's a pull request started by someone but it doesn't seems that he would like to finish it: https://github.com/aimeos/ai-controller-jobs/pull/24

Maybe you can use some files as base.

aimeos commented 4 years ago

One request regarding coding style: If a block within {} contains more than one line, the opening and closing brace should be on their own line, e.g:

if( ... ) {
    // ...
}

but

if( ... )
{
    // ...
    // ...
}

The same applies to all blocks, i.e. for, foreach, while, try/catch, etc.

alextravin commented 4 years ago

One request regarding coding style: If a block within {} contains more than one line, the opening and closing brace should be on their own line, e.g:

What about block contains 1 line? e.g.:

foreach($stars as $star)
{
    $star->move();
}

Is it okay to place a brace on their own line at that case? Don't see at my IDE an option to place a brace on it's own line if more than 1 line at a block only.

aimeos commented 4 years ago

That's OK if it can't be configured

aimeos commented 4 years ago

Perfect! :-)