search

Shopify / erb-lint

Lint your ERB or HTML files
MIT License
593 stars 114 forks source link

docs(README.md): include HardCodedString linter #278

Open francisfuzz opened 1 year ago

francisfuzz commented 1 year ago

Summary

Closes #254

This updates the README's Linters section to include the HardCodedString linter.

I verified that this linter is not enabled by default as it's configured in this part of the lib/erb_lint/runner_config.rb. The description is sourced from the pull request's OP that originally introduced the linter.

francisfuzz commented 1 year ago

Do you think there's no need for a paragraph with bad/good examples like other linters have?

@etiennebarrie - Great question! Personally, including an example would improve this PR. When I used this for my project, my team discovered that we need to provide a custom corrector and a load_path, which does the heavy lifting of actually correcting the offenses. I wish we knew about that ahead of time! 😅

Thinking aloud: could a "good" example be including a custom corrector to work with this linter?

Originally, I refrained from providing one because I did not find any guidelines for repository contributors for requiring one and I noticed that not all linters have examples (like ClosingErbTagIndent and ExtraNewline).

etiennebarrie commented 1 year ago

Can you add one for this one? Especially if you say it's particularly useful. We can add the other ones later.

francisfuzz commented 1 year ago

@etiennebarrie - Thanks for asking! I started in https://github.com/Shopify/erb-lint/pull/278/commits/1607aacc17af818f9e34335ec0871a859869aded. I'd like to verify what I wrote down works in a test repository before asking for another review.

francisfuzz commented 1 year ago

I'd like to verify what I wrote down works in a test repository before asking for another review.

Did you get a chance to try this in a test repository?

👋 @etiennebarrie - Hi there! I'll be honest: this slipped from my radar, sorry!

However, I have some time today to work on this! I created https://github.com/francisfuzz/erb-lint-playground to test things out. Thanks for your suggested changes -- I'll go ahead review+accept those now.

francisfuzz commented 1 year ago

Update! I've been working with my company internally regarding the CLA before continuing this work -- I'll follow up once I get an update from them. 🙇

samrjenkins commented 3 months ago

I just ran into this issue. Would be awesome to get this merged. Had me confused for a few minutes!