Skip to content

[ci skip] Update caching development mode default status docs#52780

Merged
rafaelfranca merged 1 commit intorails:mainfrom
starwels:patch-1
Sep 11, 2024
Merged

[ci skip] Update caching development mode default status docs#52780
rafaelfranca merged 1 commit intorails:mainfrom
starwels:patch-1

Conversation

@starwels
Copy link
Contributor

@starwels starwels commented Sep 3, 2024

Motivation / Background

This Pull Request has been created because the default state of caching on a fresh rails app is wrong in the docs

Detail

Rails.application.config.action_controller.perform_caching returns false in a fresh created rails app

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes 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 docs label Sep 3, 2024
@p8
Copy link
Member

p8 commented Sep 3, 2024 •
edited
Loading

By default :memory_store is enabled:

rails/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt

Line 33 in 5f71da1

config.cache_store = :memory_store

ActionController caching is still disabled by default though.
See: #52474 and #52474

I think the wording can be improved though...

@p8
Copy link
Member

p8 commented Sep 3, 2024 •
edited
Loading

We also need to change the following to make clear it's only for ActionController caching:

rails/railties/lib/rails/dev_caching.rb

Lines 13 to 19 in 5f71da1

if File.exist?(FILE)
delete_cache_file
puts "Development mode is no longer being cached."
else
create_cache_file
puts "Development mode is now being cached."
end

And here:

rails/railties/lib/rails/commands/dev/dev_command.rb

Line 8 in 5f71da1

desc "cache", "Toggle development mode caching on/off"

p8
p8 reviewed Sep 3, 2024
View reviewed changes
p8
p8 reviewed Sep 3, 2024
View reviewed changes
@starwels
Copy link
Contributor Author

starwels commented Sep 3, 2024 •
edited
Loading

We also need to change the following to make clear it's only for ActionController caching:

rails/railties/lib/rails/dev_caching.rb

Lines 13 to 19 in 5f71da1

if File.exist?(FILE)
delete_cache_file
puts "Development mode is no longer being cached."
else
create_cache_file
puts "Development mode is now being cached."
end

And here:

rails/railties/lib/rails/commands/dev/dev_command.rb

Line 8 in 5f71da1

desc "cache", "Toggle development mode caching on/off"

@p8 Can I make those changes in this PR or should I open a new one just for the code change?

@p8
Copy link
Member

p8 commented Sep 3, 2024

@starwels I think that if we are going to change this, we might as well fix it in all occurences.
So yes, add it to this PR please.

@rails-bot rails-bot bot added the railties label Sep 4, 2024
p8
p8 reviewed Sep 5, 2024
View reviewed changes
@p8
Copy link
Member

p8 commented Sep 5, 2024

Thanks @starwels !
Can you squash your commits?

@starwels
Update caching development mode default status docs
bfeb578 
* Update caching development mode default status docs

Update guides/source/caching_with_rails.md

Co-authored-by: Petrik de Heus <petrik@deheus.net>

Update caching_with_rails.md

Update dev_command.rb

Update dev_caching.rb

Update dev_caching.rb
@p8 p8 added the ready PRs ready to merge label Sep 10, 2024
@rafaelfranca rafaelfranca merged commit 6fbf8d0 into rails:main Sep 11, 2024
@starwels starwels deleted the patch-1 branch September 11, 2024 16:42
kamipo added a commit that referenced this pull request Sep 11, 2024
@kamipo
Fix test failure caused by #52780
6fe6e71 
https://buildkite.com/rails/rails/builds/111223#0191e1f4-0419-4a3c-9cec-01067adfd283
dhh added a commit that referenced this pull request Sep 11, 2024
@dhh
Merge branch 'main' into kamal-2
ab28238 
* main:
  Always separate config blocks with a CR
  Fixup tests for #52850
  Stop recommending Kredis
  Fix test failure caused by #52780
  Mention `assume_ssl` in SSL middleware.
DanielaVelasquez pushed a commit to DanielaVelasquez/rails that referenced this pull request Oct 3, 2024
@kamipo @DanielaVelasquez
Fix test failure caused by rails#52780
74c43fa 
https://buildkite.com/rails/rails/builds/111223#0191e1f4-0419-4a3c-9cec-01067adfd283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@p8 p8 p8 approved these changes

Assignees

No one assigned

Labels

docs railties ready PRs ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@starwels @p8 @rafaelfranca