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

Introduce suspenders:inline_svg generator #1140

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

crackofdusk
Copy link
Contributor

@crackofdusk crackofdusk commented Nov 10, 2023

Ports the existing generator to Suspenders 3.0.

To do:

  • Add documentation
  • Manually test the generator against a new Rails app

How to test manually

Add suspenders to the Gemfile.

group :development, :test do
  gem "suspenders", git: "https://github.com/thoughtbot/suspenders", branch: "suspenders-3-0-0-inline-svg-generator"
end

Install and run the generator:

bundle
bin/rails g suspenders:inline_svg

Sorry, something went wrong.

@crackofdusk crackofdusk mentioned this pull request Nov 10, 2023
17 tasks
@crackofdusk crackofdusk force-pushed the suspenders-3-0-0-inline-svg-generator branch from 07772d1 to 60a8075 Compare November 10, 2023 14:25
@crackofdusk crackofdusk changed the title WIP: Introduce suspenders:inline_svg generator Introduce suspenders:inline_svg generator Nov 10, 2023
@crackofdusk crackofdusk marked this pull request as ready for review November 10, 2023 14:31
Copy link
Contributor

@stevepolitodesign stevepolitodesign left a comment

Choose a reason for hiding this comment

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

Thank you for not only taking care of this, but live steaming it too. I just had a few comments, but nothing blocking.

class InlineSvgGenerator < Rails::Generators::Base
include Suspenders::Generators::APIAppUnsupported
source_root File.expand_path("../../templates/inline_svg", __FILE__)
desc "Installs inline_svg"
Copy link
Contributor

Choose a reason for hiding this comment

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

I should have mentioned this in the contributing section of the pull request description, but the current release has descriptions. We might want to use the existing description for consistency's sake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll change it here and in the documentation.

README.md Outdated

Installs [inline_svg]

`./bin/rails g suspenders:inline_svg`
Copy link
Contributor

Choose a reason for hiding this comment

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

[Minor]

#1141 will remove the ./. We might want to address that here too.

Suggested change
`./bin/rails g suspenders:inline_svg`
`bin/rails g suspenders:inline_svg`

README.md Outdated
@@ -30,6 +30,14 @@ Installs [capybara_accessibility_audit] and [capybara_accessible_selectors]
[capybara_accessibility_audit]: https://github.com/thoughtbot/capybara_accessibility_audit
[capybara_accessible_selectors]: https://github.com/citizensadvice/capybara_accessible_selectors

### Inline SVG

Installs [inline_svg]
Copy link
Contributor

Choose a reason for hiding this comment

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

If we end up updating the description, we might want to update it here too.

Ports the existing generator to Suspenders 3.0.
@crackofdusk crackofdusk force-pushed the suspenders-3-0-0-inline-svg-generator branch from 60a8075 to 311d4a8 Compare November 13, 2023 14:43
@crackofdusk crackofdusk merged commit 47bef29 into suspenders-3-0-0 Nov 13, 2023
2 checks passed
@crackofdusk crackofdusk deleted the suspenders-3-0-0-inline-svg-generator branch November 13, 2023 14:45
stevepolitodesign pushed a commit that referenced this pull request May 10, 2024
Ports the existing generator to Suspenders 3.0.
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.

None yet

2 participants