Skip to content

Conversation

jeeyyy
Copy link
Collaborator

@jeeyyy jeeyyy commented Jun 27, 2019

Added a test to check if content/ definition actually exists for a used glossary term in rules. In other words to check if a glossary referred actually exists as a file in /pages/glossary/*.md

@annethyme @Jym77 - could you help fix up the missing glossary items?

Failed tests report:

    glossary missing for #implicit-role, usages below:
    ┌─────────┬───────────────────────────────────────────┬────────────────┐
    │ (index) │                   name                    │      slug      │
    ├─────────┼───────────────────────────────────────────┼────────────────┤
    │    0    │ 'Role has required states and properties' │ 'rules/4e8ab6' │
    └─────────┴───────────────────────────────────────────┴────────────────┘

    glossary missing for #non-empty, usages below:
    ┌─────────┬───────────────────────────────────┬────────────────┐
    │ (index) │               name                │      slug      │
    ├─────────┼───────────────────────────────────┼────────────────┤
    │    0    │ 'Validity of HTML Lang attribute' │ 'rules/bf051a' │
    └─────────┴───────────────────────────────────┴────────────────┘

Closes issue:

@jeeyyy jeeyyy requested a review from annethyme June 27, 2019 09:26
@jeeyyy jeeyyy changed the title glossary check fix: test if definition for all referenced glossary terms exist Jun 27, 2019
@jeeyyy jeeyyy mentioned this pull request Jun 27, 2019
3 tasks
Jym77
Jym77 previously requested changes Jun 27, 2019
@jeeyyy jeeyyy requested a review from Jym77 June 27, 2019 11:07
@jeeyyy jeeyyy dismissed Jym77’s stale review June 27, 2019 11:08

The changes requested does not aim to solve the actual problem of missing glossary definitions.

@WilcoFiers
Copy link
Member

To fix the media ones: @JKODU

@Jym77
Copy link
Collaborator

Jym77 commented Jul 1, 2019

Given that (i) our definition of "semantic role" already mention implicit ones; and (ii) that in the very same rule, "explicit semantic role" is already pointing to "semantic role":
I went for the quick fix, pointing "implicit semantic role" to "semantic role".

@jeeyyy
Copy link
Collaborator Author

jeeyyy commented Jul 1, 2019

Note: All definition references are fixed, waiting for #604 before this is merged.

## Applicability

Any HTML or SVG element that has an [explicit semantic role](#semantic-role), except if the element has an [implicit semantic role](#implicit-role) that is identical to the explicit semantic role.
Any HTML or SVG element that has an [explicit semantic role](#semantic-role), except if the element has an [implicit semantic role](#semantic-role) that is identical to the explicit semantic role.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely isn't the way to solve it. I think we need a definition for this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, do we also need a definition for "explicit semantic role" which is used in the very same rule and simply links to "semantic role"?

The definition of semantic role has mention of both implicit and explicit roles, with links to further explanations. I assumed that if it was OK to link "explicit semantic role" to it, then it would also be OK for "implicit semantic role".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WilcoFiers I do not understand why it is fine to have [explicit semantic role](#semantic-role) (as was here before), but it is not fine to have [implicit semantic role](#semantic-role) Especially given that the glossary entry for semantic role mention both implicit and explicit roles, with links to more details.

Could you explain why one is OK and the other is not?
Should we rather add definitions for both explicit and implicit roles? (they are going to essentially be the same links that already exist in the definition of "Semantic Role")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. We shouldn't have linked explicit semantic role to semantic role either. I'll open an issue for it, as to not block this PR any further.

@annethyme annethyme removed their assignment Jul 9, 2019
@Jym77 Jym77 requested a review from WilcoFiers July 9, 2019 14:08
@jeeyyy
Copy link
Collaborator Author

jeeyyy commented Jul 10, 2019

@WilcoFiers can you respond on this - #634 (comment)

@jeeyyy jeeyyy self-assigned this Jul 15, 2019
@jeeyyy jeeyyy merged commit 12d14a9 into develop Jul 17, 2019
@jeeyyy jeeyyy deleted the test-if-glossary-exists branch July 17, 2019 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Additional validations for rules

4 participants