added coverage of multiple ids in tags#553
Conversation
|
|
||
| ``` | ||
| [ <covered-artifact-type> -> <specification-object-id> ] | ||
| [ <covered-artifact-type> -> <list-of-specification-object-ids> ] |
There was a problem hiding this comment.
| [ <covered-artifact-type> -> <list-of-specification-object-ids> ] | |
| [ <covered-artifact-type> -> <specification-object-id>] | |
| If a piece of code covers multiple specification items, you can also list them separated by comma. | |
| [ <covered-artifact-type> -> <specification-object-id>, <specification-object-id>] |
There was a problem hiding this comment.
Thanks @Davidius86 for the PR. I am started reviewing it. I will update main to 2.6.0 and create the release letter in another PR, so that we have a place where we can mention the change.
There was a problem hiding this comment.
Completeness: Please also add a short example of the syntax to .agents/skills/openfasttrace/SKILL.md.
There was a problem hiding this comment.
Completeness: Please add an short entry for this in the change log too (we are currently working towards 4.6.0).
| ``` | ||
|
|
||
| ##### Forwarding Requirements | ||
| ##### Needed Coverage |
There was a problem hiding this comment.
Good find. 👍
Compliance: I am guessing you are using an LLM to help with the work. Please note that according to our contribution guideline, LLM-assisted PR must state that.
There was a problem hiding this comment.
Completeness: Please also add tests that the features with requiring coverage and partial spec item definition still work in combination with the comma-separated covered IDs.
|
|
||
| private static List<String> parseCommaSeparatedList(final String input) | ||
| private static List<SpecificationItemId> parseCoveredIds(final String input) | ||
| { |
There was a problem hiding this comment.
Safety: Add null handling, please.
| final int counter) | ||
| { | ||
| final String uniqueName = this.file.getPath() + lineNumber + counter + coveredId; | ||
| final String CoveredIdStringsJoinedWithHyphen = coveredIds.stream() |
There was a problem hiding this comment.
UX: This is where the compactness gets to be a problem. While it is nice to be able to cover multiple spec ids in one line, the generated single ID from the code gets excessively long. I would suggest to instead create one "virtual" covering spec item id per covered id instead of a really long one that won't be readable.
There was a problem hiding this comment.
I'm not sure whether I got this correctly.
Does "virtual" mean that multiple new specification ids are generated, one per covered id?
That would create one entry in the trace report for each covered id of this tag.
It would also require to cover each of the specification ids derived from the covered ids.
I have only a limited knowledge on the use cases but I think I would be surprised if a tag would create more than one specification id. Especially if this behavior changes depending on whether a name has been specified or is auto generated.
You have a valid point here, the readability of the concatenated name is definitely low. The following options came to my mind:
- Use only the name of the first covered id (hides information / may be misinterpret)
- Use only a hash without any naming (decreases readability)
- Use an abbreviation (decreases readability)
Sorry, for this lengthy post. Please tell me which solution you prefer and I will adapt the PR accordingly.
2 participants
This PR enhances tags to define multiple covered ids in a tag.
ChatGPT 5.4 has been used for the first draft of the implementation and for the test.
The test was in almost in a good condition. The implementation has been heavily refactored after generation.
Documentation is written manually.