Replies: 1 comment 1 reply
-
Morning @poleguy ,
Thanks for the kind words. You do make a good point about the why, or rationale, for the rule existing. Some are fairly obvious, like single spaces or blank lines. There are several rules with a justification where I had a strong opinion beyond "It will look better":
I do agree with your statement. It would be nice to have a justification documented, then the user can assess whether there is a good reason for the rule or if it is arbitrary.
I think this thread would be a good place to continue this discussion.
One word of caution, the restructed text documentation for each rule is checked against the docstring for each rule. I wanted to ensure consistency in the applying the phase, error/warning and group labels under each rule: and to ensure I was not missing any rule documentation. I think we should decide how to make the why stand out from other information in the current rule documentation. Currently the rule documentation follows this format:
It seems you are proposing to add another element called Rationale (There is probably a better word we could use.). It seems we could locate it before the Violation example:
...or maybe before the configuration options?
Maybe it would have the same font as Violation and Fix?
What do you think? Regards, --Jeremy |
Beta Was this translation helpful? Give feedback.
-
(For context: I am a 20+ year vhdl veteran.)
Awesome tool!
I'm trying to use vhdl-style-guide on a large existing vhdl file for synthesis in Xilinx Vivado. I'm seeing a lot of signal_007 errors due to initialization of registers... at least on Xilinx devices this is the common way to set the power-on default to either 0 or 1.
I could just disable this rule... I suppose I will... but before I did that...
I looked in the documentation. I wanted to find the reason why the rule is there, so I can understand the rationale for having the rule. I'd hate to turn off the rule thinking I understood it fully, but actually failed to understand why it is a good rule to always follow, or why it might be a good rule on certain architectures, tools, etc.
I don't see the "why" questions answered in the documentation. Did I miss a place that is documenting the rationale?
I'd like to answer the "why" question for signal_007 errors and I'd like to add it to the documentation. I think this is the right place:
https://vhdl-style-guide.readthedocs.io/en/latest/signal_rules.html?highlight=signal_007#signal-007
It only answers the question of "what" the rule does. That adds little value. I'd like to change it from:
"This rule checks for default assignments in signal declarations."
to:
"This rule checks for default assignments in signal declarations. It helps avoid setting a signal to default to 1 on an architecture that does not support it that has tools that do not correctly error out when this is done, e.g. Lattice. For architectures such as Xilinx this rule should be disabled because the correct way to specify the power-on default is to set a default value on the signals."
That's my best guess, but again: I'm looking for the rationale, and didn't write the code myself.
I'd like to contribute to the documentation by changing the documentation to answer the "why" question. I find that documents that focus on "what" questions are not very valuable.
I can easily see what the rule does from the output on the terminal:
" signal_007 | Error | 1198 | Remove default assignment."
So I don't need to go to the documentation to learn that. When I go to the documentation, I want to see "why" this rule was created in the first place, what risk it is mitigating, why it's better than not having the rule, why I shouldn't disable it, etc.
I'd also like to know where the correct place to discuss the "why questions" is? Here in Q&A? Or better in a PR to change the documentation?
I'd be happy to take a whack at editing the documentation for the following, as these are the ones that are showing up for my code, and I'll want to either fix them or disable them, which means I'll have to think about the "why?" question:
signal_007 | Error | 1243 | Remove default assignment.
process_016 | Error | 1259 | Add label for process statement
process_018 | Error | 1266 | Add a label for the "end process".
instantiation_034 | Error | 1428 | Change to component instantiation
Beta Was this translation helpful? Give feedback.
All reactions