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

Fix error in the sed commands used to prefix namespaces #28038

Merged
merged 1 commit into from Oct 21, 2020

Conversation

rodrigoprimo
Copy link
Contributor

Changes proposed in this Pull Request:

This PR fixes a minor error in the sed commands used to prefix namespaces in the vendor folder. Due to an extra space, sed was
considering .bak an extra parameter. If we use -i.bak instead (without the space), sed creates a backup file, which is the desired behavior.

With this change, we won't be getting the errors below when running composer install or composer update:

Generating autoload files
> sh ./bin/prefix-vendor-namespaces.sh
Prefixing the appropriate vendor namespaces with Automattic\WooCommerce\Vendor
sed: can't read .bak: No such file or directory
sed: can't read .bak: No such file or directory
sed: can't read .bak: No such file or directory
(...)

Since sed was working without creating a backup file, I wonder if we really need to create one. I tested and sed -i -E -e "s/\"(League\\\\\\\Container)/\"Automattic\\\\\\\WooCommerce\\\\\\\Vendor\\\\\\\\\1/g" vendor/league/container/composer.json works well in Linux. If it also works on Mac, maybe we can use it and remove the code that deletes the .bak files? (cc @Konamiman)

How to test the changes in this Pull Request:

  1. Check that when running composer install or composer update, the files in vendor/league/container are correctly prefixed with Automattic\WooCommerce\Vendor and that you don't get the errors mentioned above.

This commit fixes a minor error in the sed commands used to prefix
namespaces in the vendor folder. Due to an extra space sed was
considering `.bak` an extra parameter. If we use `-i.bak` instead (without the
space), sed creates a backup file which is the desired behavior.

With this change we won't be getting the errors below when running
`composer install` or `composer update`:

```
Generating autoload files
> sh ./bin/prefix-vendor-namespaces.sh
Prefixing the appropriate vendor namespaces with Automattic\WooCommerce\Vendor
sed: can't read .bak: No such file or directory
sed: can't read .bak: No such file or directory
sed: can't read .bak: No such file or directory
(...)
```

Since sed was working without creating a backup file, I wonder if we
really need to create one. I tested and `sed -i -E -e
"s/\"(League\\\\\\\Container)/\"Automattic\\\\\\\WooCommerce\\\\\\\Vendor\\\\\\\\\1/g"
vendor/league/container/composer.json` works well in Linux. If it also
works on Mac, maybe we can use it and remove the code that deletes the
.bak files?
Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great!

@claudiosanches claudiosanches merged commit 600d489 into master Oct 21, 2020
@claudiosanches claudiosanches deleted the fix/minor-error-in-prefix-vendor-namespaces branch October 21, 2020 13:13
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Oct 21, 2020
@claudiosanches claudiosanches removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Oct 21, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants