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

tmp:create creates the same dirs that tmp:clear removes #48057

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jasonkarns
Copy link
Contributor

After running tmp:clear, tmp:create should ensure the same folders that are cleared are created.

Motivation / Background

This Pull Request has been created because the current tmp:clear and tmp:create tasks are opposites of each other, but do not currently affect the same directories. tmp:clear presently clears cache, sockets, screenshots and storage, but tmp:create only creates the directories for cache, sockets, and pids.

Thus there is some incongruency between tmp:clear and tmp:create. Running tmp:create, for instance, does not result in a tmp/storage/ directory and thus any ActiveStorage tests must create the directory themselves.

Detail

This Pull Request changes the tmp:create rake task to additional ensure screenshots and storage directories are created.

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the railties label Apr 25, 2023
@al2o3cr
Copy link
Contributor

al2o3cr commented Apr 25, 2023

FWIW, in a new Rails app, tmp/storage (and tmp/storage/.keep) are generated automatically when an app is generated unless the --skip-active-storage flag is given:

empty_directory_with_keep_file "tmp/storage"

@al2o3cr
Copy link
Contributor

al2o3cr commented Apr 25, 2023

The description of tmp:clear and friends in the guide will also need updating.

@jasonkarns
Copy link
Contributor Author

I'm also wondering if it was an oversight that the "root" tmp:clear task doesn't clear pids.

@rails-bot rails-bot bot added the docs label Apr 25, 2023
After running tmp:clear, tmp:create should ensure the same folders that
are cleared are created.
@skipkayhil
Copy link
Member

I think if we want to improve tmp commands we should probably go through #47698.

Instead of always creating storage and screenshots, it should probably depend on whether the app loads Active Storage and whether the app has system tests configured.

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

Successfully merging this pull request may close these issues.

None yet

3 participants