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

Fix attribute formatting from running erb-lint #269

Closed
wants to merge 6 commits into from

Conversation

jrussell416
Copy link
Collaborator

@jrussell416 jrussell416 commented Feb 4, 2025

This PR addresses formatting issues identified after implementing the ERB linter in #264, which was introduced to address #175.

This fix added quotes around attribute value

Before
Screenshot 2025-02-04 120616

After
Screenshot 2025-02-04 120603

During PR tests, RuboCop flagged some offenses related to bin/erb and StringLiterals. . These issues were fixed using RuboCop's --auto-correct option to ensure the PR passes all tests.

@jrussell416 jrussell416 changed the title Fix/erb issue attribute Fix attribute formatting from running erb-lint Feb 4, 2025
.gitignore Outdated
# ERB Lint binaries
echo "bin/erb-lint" >> .gitignore
echo "bin/erb_lint" >> .gitignore
echo "bin/erblint" >> .gitignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I would just add the erb lint filenames directly, rather than going via echo

<%= yield %>
<div id="root" class="min-w-full">
<div id="root" class="min-w-full"></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice spot, thanks!

@@ -0,0 +1 @@
/home/jennifer/.rbenv/versions/3.3.4/lib/ruby/gems/3.3.0/gems/erb_lint-0.8.0/exe/erb_lint
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels like it should not be checked in with your path in it

Copy link
Collaborator

@sarah-fflow sarah-fflow left a comment

Choose a reason for hiding this comment

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

See the two file comments

@jrussell416 jrussell416 reopened this Feb 24, 2025
@jrussell416
Copy link
Collaborator Author

Reviewing the conflicts, I realized that the underlying issue was related to the binstub setup on my local. Running erb_lint wasn't working consistently across branches, which led me to try a solution involving bundle binstubs erb_lint to create a local binstub (bin/erb-lint).

However, this approach inadvertently caused issues with the bin directory and .gitignore. Now that I’ve resolved the binstub problem, I’ll redo this PR with a cleaner approach to ensure consistency moving forward.

Closing this PR to open a new one with the correct fixes.

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