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.

Sign up for GitHub

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

Jump to bottom

fix(dev): merge 'assets' and 'output' paths #220

Merged
merged 7 commits into from
Sep 28, 2018
Merged

fix(dev): merge 'assets' and 'output' paths #220

merged 7 commits into from
Sep 28, 2018

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented Aug 10, 2018

Fix the discrepancy between production and development mode paths which causes one or both to 404 on scripts. Do so by simplifying setup to just a single output directory.

This PR is a:

[ ] New feature
[ ] Enhancement/Optimization
[ ] Refactor
[x] Bugfix
[ ] Test for existing code
[ ] Documentation

Summary

When this pull request is merged, it will...

  • Remove the concept of the "assets" directory entirely, and set the
    "output" directory to be just "web/".
  • Remove the old logo asset from /web/ (pre-Venia rebrand!)
  • Gitignore entire "web" directory
  • Set JS files to output to 'web/js' subdirectory by adding directory
    prefix to output.filename and output.chunkFilename
  • Modify tests, documentation, and type expectations to remove
    paths.assets

@zetlen zetlen requested review from jimbo and jcalcaben August 10, 2018 21:58
fix(dev): merge 'assets' and 'output' paths
36d8157 
- Remove the concept of the "assets" directory entirely, and set the
"output" directory to be just "web/".
- Remove the old logo asset from /web/ (pre-Venia rebrand!)
- Gitignore entire "web" directory
- Set JS files to output to 'web/js' subdirectory by adding directory
prefix to `output.filename` and `output.chunkFilename`
- Modify tests, documentation, and type expectations to remove
`paths.assets`
@zetlen zetlen force-pushed the zetlen/refactor-output-paths branch from 3fb7816 to 36d8157 Compare August 10, 2018 22:27
@coveralls
Copy link

coveralls commented Aug 10, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 1045

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 31.052%
Totals Coverage Status
Change from base Build 1044: 0.0%
Covered Lines: 536
Relevant Lines: 1779
💛 - Coveralls

@zetlen zetlen mentioned this pull request Aug 11, 2018
Merged
@zetlen zetlen changed the base branch from master to release/1.0 September 25, 2018 20:40
@zetlen zetlen changed the base branch from release/1.0 to release/2.0 September 26, 2018 21:10
@zetlen
Copy link
Contributor Author

zetlen commented Sep 27, 2018

@jimbo Since the concern about root.phtml no longer applies to the UPWARD architecture change, can I ask if you have any other reviewable concerns, or is this one good to go?

@zetlen zetlen merged commit 6075d22 into release/2.0 Sep 28, 2018
@jimbo jimbo deleted the zetlen/refactor-output-paths branch October 1, 2018 17:40
@jimbo jimbo mentioned this pull request Oct 1, 2018
Merged
This was referenced Mar 1, 2023
Open
Open
@sharkySharks sharkySharks mentioned this pull request Nov 28, 2023
Open
@rtsehynka-gm rtsehynka-gm mentioned this pull request Nov 28, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimbo jimbo jimbo requested changes

@jcalcaben jcalcaben Awaiting requested review from jcalcaben

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zetlen @coveralls @jimbo