jeremiah-c-leary / vhdl-style-guide

Style guide enforcement for VHDL
GNU General Public License v3.0
192 stars 40 forks source link

Issue#1273: Added rules to enforce `in` keywords in for loops/generates #1274

Closed JHertz5 closed 1 month ago

JHertz5 commented 1 month ago

Resolves #1273.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.21%. Comparing base (baf22da) to head (497c641). Report is 49 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1274 +/- ## ========================================== + Coverage 94.01% 94.21% +0.19% ========================================== Files 1557 1607 +50 Lines 29028 29809 +781 Branches 3414 3487 +73 ========================================== + Hits 27291 28084 +793 + Misses 1303 1294 -9 + Partials 434 431 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jeremiah-c-leary commented 1 month ago

Morning @JHertz5 ,

I was curious why you used two rules instead of a single parameter_specification rule to set the case of in?

--Jeremy

JHertz5 commented 1 month ago

Hi @jeremiah-c-leary.

I wanted rules to cover both for-generates and for-loops, and thought it may be desirable to handle them separately and use the existing rule groups. This is similar to the way in which, for example, case_generate_alternative_501 checks the case on choice.others_keyword tokens, but only within a case generate construct.

Would you prefer for me to replace the new rules with a single rule in a new rule group for parameter_specification? I'm happy to do so if that's the case.

Thanks Jukka

jeremiah-c-leary commented 1 month ago

Morning @JHertz5 ,

Would you prefer for me to replace the new rules with a single rule in a new rule group for parameter_specification? I'm happy to do so if that's the case.

I do not have a strong opinion and your reasoning is sound. You never know if someone would like a different case based on the context. This is something I struggle with sometimes.

I will get this merged to master.

--Jeremy