Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyzer): rule useHtmlLang #4052

Merged
merged 3 commits into from
Dec 21, 2022

Conversation

mrkldshv
Copy link
Contributor

Summary

Closes #3944.

Test Plan

Run newly added tests.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Dec 13, 2022

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 646cf7e
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/639d9b077edf9200093c6895

@mrkldshv
Copy link
Contributor Author

In general rule works as expected but I feel some code can be simplified/refactored. I'd appreciate any suggestions!

@mrkldshv
Copy link
Contributor Author

@ematipico Thanks for the review! I'll adjust code where necessary and push the changes.

Copy link
Contributor

@leops leops left a comment

Choose a reason for hiding this comment

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

Overall I think the rule is correct, but maybe it would make sense to merge its functionality into the similar useValidLang rule ?

@ematipico
Copy link
Contributor

Overall I think the rule is correct, but maybe it would make sense to merge its functionality into the similar useValidLang rule ?

Yeah, I think it makes sense. We created two rules because the eslint plugin has two rules, so we tried not to diverge too much. We can easily merge the two rules into one later.

@ematipico ematipico merged commit 1d4325d into rome:main Dec 21, 2022
@mrkldshv mrkldshv deleted the feature/use-html-lang branch December 21, 2022 22:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useHtmlLang
3 participants