svlint icon indicating copy to clipboard operation
svlint copied to clipboard

Add fuller explanations about each rule.

Open DaveMcEwan opened this issue 2 years ago • 3 comments

  • Add a new member explanation to each rule.
  • Adjust the formatting of RULES.md (via mdgen) to be slightly easier to read.
  • Adjust hint and reason to be full American English sentences with consistent in position and tense.
  • Write full explanations of reasoning behind each rule (in progress).

@dalance Before I spend too much time on documentation, are you okay with this? All TODOs will be addressed before this is moved from "draft" status.

DaveMcEwan avatar May 12 '22 11:05 DaveMcEwan

Thank you for a lot of your work. I think it's good.

dalance avatar May 13 '22 09:05 dalance

@dalance There are a few rules which I think could be named more consistently to help new users find them in alphabetical order. Would you accept these renames (using the existing rename mechanism in build.rs)?

  • generate_keyword_required -> keyword_required_generate
  • generate_keyword_forbidden -> keyword_forbidden_generate
  • legacy_always -> keyword_forbidden_always
  • priority_keyword -> keyword_forbidden_priority
  • unique0_keyword -> keyword_forbidden_unique0
  • unique_keyword -> keyword_forbidden_unique
  • wire_reg -> keyword_forbidden_wire_reg

Looking ahead, I'm planning a (separate) PR to add some rules to perform regex checks on various objects. To enforce linear runtime, I'm planning a pair of rules for each object like re_required_module and re_forbidden_module. So the above renames would fit nicely with that.

Another PR I'd like to do is to add some more rules forbidding things like inside, property, defparam, and some others which don't make sense in certain circumstances. So the above renames allow natural extension like keyword_forbidden_defparam.

I appreciate I should have asked about the renames already in this PR (for_with_begin -> multiline_for_begin and if_with_begin -> multiline_if_begin). If you think they're inappropriate, I can undo those renames before finishing this PR.

DaveMcEwan avatar Oct 04 '22 17:10 DaveMcEwan

@DaveMcEwan The new naming rule seems to be more consistent than now. I agree it.

dalance avatar Oct 04 '22 23:10 dalance

@dalance I think this is finally usable. There will likely be corrections and additions in future, but at least it's easier to change existing documentation rather than starting blank :p The description at the top of the PR is updated to cover all the salient points.

DaveMcEwan avatar Oct 28 '22 18:10 DaveMcEwan

Sorry for the late reply. All changes seem to be good. I'll merge and release a new version.

dalance avatar Nov 06 '22 23:11 dalance