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

[Backport for 5.x] Keep backups, even when they're old #2912

Merged
merged 1 commit into from Apr 27, 2021
Merged

[Backport for 5.x] Keep backups, even when they're old #2912

merged 1 commit into from Apr 27, 2021

Conversation

Pezmc
Copy link

@Pezmc Pezmc commented Feb 4, 2021

That is a backport for 5.x versions of #2734

It fixed an issue where webpacker would remove previous assets during clean, rather than keeping the specified number of backups and is currently only available in the 6.x beta.

Our rails projects are still currently using a custom fork of webpacker with this fix applied, but it seems many others have unearthed the same problem (#2443 (comment), Quimbee@624f687) backporting will fix up the issue for those yet to, or unable to, upgrade to 6.x.

/cc: @dkniffin @viniciusgama @braddunbar @oboxodo

Recently, we encountered an issue with webpacker removing our previous assets
during the webpacker:clean task, and not keeping the specified number of
backups. We expected the [behavior from sprockets][sprockets], in which assets are
kept if under age or within the count limit. However, webpacker only keeps
assets if under age *AND* within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

[sprockets]: https://github.com/rails/sprockets/blob/358f83ff09a77f69ac17543a9b1d127737060f00/lib/sprockets/manifest.rb#L258-L259

Backport-PR-URL: #2912
@oboxodo
Copy link

oboxodo commented Feb 4, 2021

@Pezmc you beat me to it! hahaha! I had cherry-picked this commit to my fork yesterday and was planning to send the PR today. Thanks!

@pedrofurtado
Copy link
Member

pedrofurtado commented Feb 9, 2021

Hey, @Pezmc ! 👋

Thanks for backport this bugfix in 5.x branch 🤝

I think we are ready to merge @gauravtiwari

PS: The failed builds in CI is not related to the PR code, and I opened a PR to fix that: #2903

@Pezmc
Copy link
Author

Pezmc commented Feb 26, 2021

@pedrofurtado @gauravtiwari How can I help get a release cut here?

6.0 is slated to be released soon which makes this backport semi-redundant, it'd be nice to benefit from it in the mean time and move my projects away from our custom fork of webpacker.

@Pezmc Pezmc mentioned this pull request Mar 18, 2021
@gauravtiwari gauravtiwari merged commit eedd2ab into rails:5-x-stable Apr 27, 2021
@gauravtiwari
Copy link
Member

released in 34c02ff

thank you 🙏

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

5 participants