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

Docs: Update the module webpacker migration guide #8180

Merged
merged 2 commits into from Jul 1, 2021

Conversation

ahukkanen
Copy link
Contributor

🎩 What? Why?

This updates the Webpacker migration guide for Decidim modules regarding the new conventions available in the module's assets configuration file (config/assets.rb).

πŸ“Œ Related Issues

Testing

Read the docs and see if you understand.

πŸ“‹ Checklist

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • βœ”οΈ DO check open PR's to avoid duplicates.
  • βœ”οΈ DO keep pull requests small so they can be easily reviewed.
  • βœ”οΈ DO build locally before pushing.
  • βœ”οΈ DO make sure tests pass.
  • βœ”οΈ DO make sure any new changes are documented in docs/.
  • βœ”οΈ DO add and modify seeds if necessary.
  • βœ”οΈ DO add CHANGELOG upgrade notes if required.
  • βœ”οΈ DO add to GraphQL API if there are new public fields.
  • βœ”οΈ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

Copy link
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

Awesome job!! 😍 πŸ‘

----
Decidim.register_component(:your_component) do |component|
component.engine = Decidim::YourComponent::Engine
component.stylesheet = "decidim/your_component/your_component"
Copy link
Contributor

Choose a reason for hiding this comment

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

I've found that several components are still assigning the stylesheet attribute. I understand that this is not needed anymore, nor the admin_stylesheet attribute, right?
I'm thinking of creating a PR removing those attributes and all the assignments in the component manifests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, those attributes no longer have any effect.

If you are planning to remove these attributes from the Decidim::ComponentManifest class, I suggest you also create replacement methods for stylesheet=(_ss) and admin_stylesheet=(_ss) that would throw more explanatory errors for the user guiding them to the migration guide.

@leio10 leio10 merged commit 08879eb into decidim:develop Jul 1, 2021
@ahukkanen ahukkanen deleted the doc/webpacker-updates branch July 1, 2021 16:30
@andreslucena andreslucena added type: internal PRs that aren't necessary to add to the CHANGELOG for implementers and removed type: enhancement target: developer-experience labels Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: core type: internal PRs that aren't necessary to add to the CHANGELOG for implementers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants