change letter case for test case identifier#1200
Conversation
marc-vanderwal
left a comment
There was a problem hiding this comment.
I only have one little suggestion for one paragraph.
| All test cases belong to one module, and it gets its name from that module's | ||
| name. The following modules are defined and available: |
There was a problem hiding this comment.
| All test cases belong to one module, and it gets its name from that module's | |
| name. The following modules are defined and available: | |
| All test cases belong to one module and their names are based on that module’s name. | |
| The following modules are defined and available: |
There was a problem hiding this comment.
I changed the wording to match your proposal
matsduf
left a comment
There was a problem hiding this comment.
Instead of recommendation I think we should have a "should".
| When referencing to a test case the test level name may be used in other letter | ||
| case than all uppercase. | ||
| When referencing to a test case the module name may be used in other letter | ||
| case, but for readability, camel case sould be prefered. |
There was a problem hiding this comment.
I do not think these are examples of "camelCase", https://en.wikipedia.org/wiki/Camel_case
"sould" --> "should"
There was a problem hiding this comment.
I changed it for PascalCase then.
The typo has been fixed.
There was a problem hiding this comment.
No, "Address" is not PascalCase.
There was a problem hiding this comment.
See my other comment.
| The module name is not case sensitive, but camel case, as defined above, | ||
| should be prefered. |
There was a problem hiding this comment.
The module name is not case sensitive, but the forms defined above,
should be used when referring to the modules, i.e. "Address" and neither
"ADDRESS" nor "address".
| * Zone06 | ||
|
|
||
| One exceptional test case identifier is `BASIC00`, in which the index | ||
| One exceptional test case identifier is `Basic00`, in which the index |
There was a problem hiding this comment.
Basic00 has been removed, hasn't it?
matsduf
left a comment
There was a problem hiding this comment.
This document defines the case of the test level identifiers, and I think that is good, but I would like to see stronger wording.
It also tries to rename "test level" to "module", and that might be good, but it requires more changes and it should not start here. Rather in the master test plan.
In the description it is referred to "Camel case", but none of the suggestions are Camel case. "iPhone" is example of Camel case, but "Address" is not.
| All test cases belong to one module and their names are based on that module’s | ||
| name. The following modules are defined and available: |
There was a problem hiding this comment.
This document now refer "modules" but previously it referred to "test levels". We should use the same terminology in all places. The reason why this document referred to "test level" is because it is called so in other documents. We should be consistent.
There was a problem hiding this comment.
I think the current wording across component is Module,
- Test zone data : https://github.com/zonemaster/zonemaster/blob/397c1281da08d0f3323fae6eae1ab54ec0779838/test-zone-data/README.md?plain=1#L94C1-L96C62
- Backend RPCAPI document :
- CLI usage : https://github.com/zonemaster/zonemaster-cli/blob/master/lib/Zonemaster/CLI.pm#L119-L127
and so on.
The wording "module" is used quite a lot, see Engine, CLI and Backend implementation and documentation. I just tried to make this document more uniform with the rest of the documents / components.
I changed it to refers to PascalCase not camelCase. "Address" is PascalCase, with one word it would just be equivalent to the capitalized word, but covers the case of something with several words without having to change the document in the future. |
|
It is obvious that there is an inconsistency between "test level" and "module". The test plans refer to the IEEE terminology "test level". I guess that "module" has come from the Perl programming. I am not against leaving "test level" but we should consider if there is any problem of using "module" for the non-programming side of Zonemaster. In the code, module also refer to other parts that do not match any test levels. If we decide to do the change, we should also update other documents to have a complete switch. When it comes to Pascal case, I cannot see that any of the names of the test levels (or modules) are to be written in Pascal case. I do not think we should use the term in the document. It does not add anything, and it makes the text harder to understand. "The more specific terms Pascal case and upper camel case refer to a joined phrase where the first letter of each word is capitalized, including the initial letter of the first word." https://en.wikipedia.org/wiki/Camel_case |
|
All comments should have been addressed. |
|
@matsduf please re-review, just fixed a typo. |
Purpose
Improve test case identifier and module readability by preferring capitalized words to upper case.
Context
zonemaster/zonemaster-gui#437
Changes
How to test this PR
-