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

fixes webpacker:clean erroring because of nested hashes on manifest #2318

Merged
merged 1 commit into from
Nov 11, 2019
Merged

fixes webpacker:clean erroring because of nested hashes on manifest #2318

merged 1 commit into from
Nov 11, 2019

Conversation

LuanGB
Copy link
Contributor

@LuanGB LuanGB commented Oct 7, 2019 •
edited
Loading

Fixes #2310

@LuanGB
Copy link
Contributor Author

LuanGB commented Oct 7, 2019

@jakeNiemiec I believe these changes are a bit more idiomatic than the previous added pr, plus it includes at least one test to ensure it won't happen again.

@swrobel
Copy link
Contributor

swrobel commented Oct 14, 2019

FYI, this should be merged before a new release is cut, as Heroku is waiting on this being functional & stable to add webpacker caching support, which would really speed up build times: heroku/heroku-buildpack-ruby#892

@LuanGB LuanGB force-pushed the webpacker-clean_with_hashes_on_manifest branch from 90cdc1b to 625a5ec Compare October 15, 2019 13:19
swrobel
swrobel reviewed Oct 15, 2019
View reviewed changes
lib/webpacker/commands.rb Outdated Show resolved Hide resolved
swrobel
swrobel reviewed Oct 15, 2019
View reviewed changes
lib/webpacker/commands.rb Outdated Show resolved Hide resolved
@gauravtiwari
Copy link
Member

Hi @LuanGB Thanks for the fix. @swrobel thanks for reviewing.

Could you please review and address the comments? Happy to merge this in once the comments are resolved.

swrobel
swrobel suggested changes Oct 30, 2019
View reviewed changes
Copy link
Contributor

@swrobel swrobel left a comment

Choose a reason for hiding this comment

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

Please see comments

@gauravtiwari gauravtiwari mentioned this pull request Nov 6, 2019
Closed
@LuanGB
Copy link
Contributor Author

LuanGB commented Nov 7, 2019

@gauravtiwari and @swrobel I'll probably be looking into this today to fix the underlined issues

@LuanGB LuanGB force-pushed the webpacker-clean_with_hashes_on_manifest branch from 625a5ec to e04cd3a Compare November 7, 2019 19:19
@LuanGB LuanGB force-pushed the webpacker-clean_with_hashes_on_manifest branch from e04cd3a to 0c7a9f7 Compare November 7, 2019 19:20
@LuanGB
Copy link
Contributor Author

LuanGB commented Nov 7, 2019

@jakeNiemiec @gauravtiwari and @swrobel I think it's good to go now!

@swrobel
Copy link
Contributor

swrobel commented Nov 7, 2019

lgtm

@saiqulhaq
Copy link

please merge

@kikonen
Copy link

kikonen commented Nov 10, 2019 •
edited
Loading

I think I hit this issue also (as part of my first experiment with this webpacker gem...).

Any ETA when release including fix would be available?

Tried out fix. It seems that it fixed immediate visible problem (deployment was failing).

kikonen added a commit to kikonen/host that referenced this pull request Nov 10, 2019
@kikonen
FIX try path for webpacker failure
1e32368 
- rails/webpacker#2318
@gauravtiwari gauravtiwari merged commit 5da39a1 into rails:master Nov 11, 2019
@gauravtiwari gauravtiwari mentioned this pull request Nov 12, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swrobel swrobel swrobel requested changes

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

Successfully merging this pull request may close these issues.

TypeError: no implicit conversion of Hash into String (Assets clean)
5 participants
@LuanGB @swrobel @gauravtiwari @saiqulhaq @kikonen