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

Updates to upgrade guide #81

Merged
merged 8 commits into from
Mar 16, 2022
Merged

Updates to upgrade guide #81

merged 8 commits into from
Mar 16, 2022

Conversation

stevegeek
Copy link
Contributor

Hi all, here are some suggested edits and additions for the upgrade documentation.

I decided on these by running through the steps starting with a fresh Rails app (generated with 6.1 then upgraded to 7)

I started with webpacker and sass-rails and tested the original steps. From that I noted something extra in relation to the default module resolution in webpacker and added a section about it.

I then also ran through a setup where CSS was built by webpacker (extract_css: true). The additional instructions didn't seem to me to warrant a separate guide so I simply modified the main one.

Happy to add/remove/modify anything as needed!

(Note the comment I made on the #76 about an error when removing webpacker was my bad)

UPGRADING.md Outdated Show resolved Hide resolved
UPGRADING.md Outdated Show resolved Hide resolved
@stevegeek
Copy link
Contributor Author

Assuming all else is ok with these changes, I will wait till #79 is merged and integrate change with this, then open the PR.

@brenogazzola
Copy link
Collaborator

It's merged.

@stevegeek
Copy link
Contributor Author

Great, I will get on this today

@stevegeek stevegeek marked this pull request as ready for review March 11, 2022 12:28
@brenogazzola
Copy link
Collaborator

@stevegeek Do you still want to add anything or can I merge this?

@stevegeek
Copy link
Contributor Author

@brenogazzola Feel free to merge, Im happy with these changes for now, if I notice anything else in future I will open a new PR.

@brenogazzola
Copy link
Collaborator

Great! Thanks a lot for helping with the guide.

@brenogazzola brenogazzola merged commit f9e8c3b into rails:main Mar 16, 2022
@stevegeek
Copy link
Contributor Author

@brenogazzola you are welcome!

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.

2 participants