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

[FrameworkBundle] Support use of hyphen in asset package name #30007

Merged
merged 2 commits into from Jan 29, 2019

Conversation

Projects
None yet
5 participants
@XuruDragon
Copy link
Contributor

XuruDragon commented Jan 28, 2019

This PR is a continuity of #28128

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29122
License MIT
Doc PR n/a

According to issue symfony/symfony-docs#10442, we tested in a demo bundle, for example in src/AppBundle/Resources/config/config.yml a package using hyphens: app-client-frontend, and withouth the patch it fails because the package is not recognized. With the patch, it works as expected.

framework:
    assets:
        packages:
            app-client-frontend:
                version: "%env(FRONTEND_VERSION)%"
                version_format: '%%2$s/dist/%%1$s'
                base_urls:
                  - "%env(FRONTEND_URL)%"

@nicolas-grekas nicolas-grekas added this to the next milestone Jan 28, 2019

@nicolas-grekas nicolas-grekas changed the title Fix #28122 Support use of hyphen in asset package name [FrameworkBundle] Support use of hyphen in asset package name Jan 28, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:pr-28122 branch from 8c0a684 to 71ff965 Jan 28, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 28, 2019

What about rebasing on 3.4 and consider it a bug fix?

damaya and others added some commits Aug 3, 2018

Support use of hyphen in asset package name
| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes (Manual tests only)
| Fixed tickets | #28122
| License       | MIT
| Doc PR        | n/a

According to issue symfony/symfony-docs#10442, we tested in a demo bundle, for example in src/AppBundle/Resources/config/config.yml a package using hyphens: app-client-frontend, and withouth the patch it fails because the package is not recognized. With the patch, it works as expected.
```
framework:
    assets:
        packages:
            app-client-frontend:
                version: "%env(FRONTEND_VERSION)%"
                version_format: '%%2$s/dist/%%1$s'
                base_urls:
                  - "%env(FRONTEND_URL)%"
```

@XuruDragon XuruDragon force-pushed the XuruDragon:pr-28122 branch from 71ff965 to 5cbc8d0 Jan 28, 2019

@XuruDragon XuruDragon changed the base branch from master to 3.4 Jan 28, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:pr-28122 branch from 5cbc8d0 to 5c58b6e Jan 28, 2019

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

XuruDragon commented Jan 28, 2019

@nicolas-grekas rebase on 3.4 is done

@fabpot

fabpot approved these changes Jan 29, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Jan 29, 2019

Thank you @XuruDragon.

@fabpot fabpot merged commit 5c58b6e into symfony:3.4 Jan 29, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Jan 29, 2019

bug #30007 [FrameworkBundle] Support use of hyphen in asset package n…
…ame (damaya, XuruDragon)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Support use of hyphen in asset package name

This PR is a continuity of #28128

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29122
| License       | MIT
| Doc PR        | n/a

According to issue symfony/symfony-docs#10442, we tested in a demo bundle, for example in src/AppBundle/Resources/config/config.yml a package using hyphens: app-client-frontend, and withouth the patch it fails because the package is not recognized. With the patch, it works as expected.
```yaml
framework:
    assets:
        packages:
            app-client-frontend:
                version: "%env(FRONTEND_VERSION)%"
                version_format: '%%2$s/dist/%%1$s'
                base_urls:
                  - "%env(FRONTEND_URL)%"
```

Commits
-------

5c58b6e Add PackageNameTest to ConfigurationTest also add in the changelog the corresponding entry to this PR
30b6a4f Support use of hyphen in asset package name

@XuruDragon XuruDragon deleted the XuruDragon:pr-28122 branch Jan 29, 2019

This was referenced Jan 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment