Added support and readme for environment variables

semaio / Magento2-ConfigImportExport

Import/Export configuration data in Magento 2 via CLI.

Open Software License 3.0

157 stars 59 forks source link

Added support and readme for environment variables #48

Closed peterjaap closed 2 years ago

peterjaap commented 2 years ago

Please make sure these boxes are checked before submitting your PR - thank you!

[x] Pull request is based against develop branch - no, there is no develop branch
[x] README.md reflects changes (if applicable)
[x] New files contain a license header

Issue

This PR fixes issue #20.

Proposed changes

I took the existing PR #21 and processed @ktomk his suggestions, and added a description to the docs.

DavidLambauer commented 2 years ago

To me, this looks perfectly fine. We don't need to overcomplex the whole thing. It is a new feature, so there shouldn't be any issues regarding BC breaks. Maybe @therouv has another opinion, but I'd say merge it and move along.

@peterjaap thank you for this feature contribution. No one said it before :(

therouv commented 2 years ago

Hi @peterjaap really happy about your contribution (incl. docs and tests) – THANKS! Changes have been merged and new release was tagged.

ktomk commented 2 years ago

You may not have noticed it, but there could be other considerations not yet realized with the regular expression changes while you decided to stop reviewing the changes. Probably you were a bit hasty not fully tracking the longer history of this feature with the recent changes, but again, take care. @DavidLambauer

peterjaap commented 2 years ago

Thanks @DavidLambauer @therouv

@ktomk thanks for your valuable input, it made this PR definitely better. We'll stay on the lookout for future considerations, but at a certain point with open source it's definitely YMMV.

DavidLambauer commented 2 years ago

too hasty? That's me ;) Nothing has changed @ktomk