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

Modernize scaffold generator #41210

Merged
merged 99 commits into from Feb 4, 2021
Merged

Modernize scaffold generator #41210

merged 99 commits into from Feb 4, 2021

Conversation

dhh
Copy link
Member

@dhh dhh commented Jan 22, 2021

The default scaffold generator should be encouraging the use of partials. It also shouldn't try so hard to style things. We can do such styling as a build-on-top via CSS framework gems, like tailwindcss-rails.

Co-authored-by: Abhay Nikam nikam.abhay1@gmail.com

To prevent conflicts with utility frameworks that might also be resetting base elements.
Shows the usage of partials right from the start. Better compatibility with upgrading to Turbo frames/stream updates.
@dhh dhh added this to the 7.0 milestone Jan 22, 2021
@rails-bot rails-bot bot added the railties label Jan 22, 2021
@dhh
Copy link
Member Author

dhh commented Jan 22, 2021

If anyone would like to help me wrap this up with correcting tests, I'd appreciate it!

@Diogomartf
Copy link

If anyone would like to help me wrap this up with correcting tests, I'd appreciate it!

I would like to help, but how do I know which tests I need to correct? All that are failing?
There are some that don't seem related.

Sorry if that might be obvious but it's my first time here ✌️

@dhh
Copy link
Member Author

dhh commented Jan 22, 2021

Thanks @Diogomartf! Yes, basically get this PR passing. There are some failing tests in the general suite that won't stop the overall build from passing. You can ignore that. Just all the ones focused on these changes ✌️❤️

…youts/application.html.erb.tt

Co-authored-by: Haroon Ahmed <haroon.ahmed25@gmail.com>
dhh and others added 3 commits January 24, 2021 13:07
Co-authored-by: Haroon Ahmed <haroon.ahmed25@gmail.com>
Co-authored-by: Haroon Ahmed <haroon.ahmed25@gmail.com>
rafaelfranca and others added 12 commits January 28, 2021 15:08
`return_only_media_type_on_content_type` will be introduced in Rails 6.2
so the changing of returning Content-Type will happen in a future
version of Rails (probably 7.0).
This reverts commit 9d8ff32, reversing
changes made to 9cde02e.

Need to revert this so I can revert another PR.
…s-enabled"

This reverts commit c97f1f1, reversing
changes made to ac7851e.

We haven't quite tracked down why yet, but this change caused our API to
not use the query cache. Our API is Sinatra mixed with Rails. We install
the Exectutor in our API so it was installed. However, (some?)
production requests were showing 0 query cache hits.

jhawthorn found that we likely need this check because if we don't the
pools returned will be a different set. He'll send a test later today.
Before this commit, Rails test Rake tasks only load the test files, and
the tests only run in an at_exit hook via minitest/autorun.

This prevents conditionally running tasks only when tests pass, or even
more simply in the right order. As a simple example, if you have:

task default: [:test, :rubocop]

The rubocop task will run after the test task loads the test files but
before the tests actually run.

This commit changes the test Rake tasks to shell out to the test runner
as a new process.

This diverges from previous behavior because now, any changes made in
the Rakefile or other code loaded by Rake won't be available to the
child process. However this brings the behavior of `rake test` closer to
the behavior of `rails test`.

Co-authored-by: Adrianna Chang <adrianna.chang@shopify.com>
@dhh dhh merged commit 164c2f6 into main Feb 4, 2021
@dhh dhh deleted the modernize-scaffold branch February 4, 2021 11:26
abhaynikam added a commit to abhaynikam/rails that referenced this pull request Feb 4, 2021
@@ -38,7 +38,6 @@

```ruby
class User < ApplicationRecord
has_many :bookmarks
Copy link
Contributor

Choose a reason for hiding this comment

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

The association was used below to demonstrate usage of strict_loading on a Model. Is the removal intentional or removed by mistake while rebasing the PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

mistake, adding back

Copy link
Contributor

Choose a reason for hiding this comment

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

Raised a PR for it: #41329. Please feel free to close it if you have added the change back 😊

@yshmarov
Copy link

yshmarov commented Mar 1, 2021

can't wait to use the new scaffold!

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