Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCR-39-ui-serials-management evaluation #69

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

maccabeelevine
Copy link
Member

@maccabeelevine maccabeelevine commented Feb 28, 2024

TC provisionally approved this module on 2024-03-18. TC and the dev team unanimously agreed that the dev team will fix the major (and critical) code smells and get the test coverage to the 80% threshold, and that it should be doable in around 2 weeks. The dev team suggested and TC agreed that substantive test cases should be prioritized rather than hitting 80% as quickly as possible.

@maccabeelevine maccabeelevine marked this pull request as ready for review March 19, 2024 14:31
@maccabeelevine maccabeelevine requested a review from a team as a code owner March 19, 2024 14:31
@maccabeelevine
Copy link
Member Author

When the Technical council's decision is contrary to the evaluation, or after any provisional acceptance or deferral, the Technical Council is required to provide written justification for their decision.

TC approved the module despite the four failing criteria because:

  1. The third-party dependency licensing issues were issues with upstream Stripes modules, and not with ui-serials-management itself. TC earlier decided to raise that issue with #stripes and then to consider changes to the process next time we do a TCR Process Improvements cycle.
  2. The WCAG compliance issues are also apparently upstream issues, not inherent in this module. The dev team has agreed to address them with the stripes team. From the self-eval:
  • We have addressed the majority of issues reported by the axe DevTools Chrome Extension. However there are still two outstanding issues, both of the same type (MCL rows generating the issue: "Certain ARIA roles must contain particular children"):
    * UISER-71
    * UISER-75
  • We see this issue in other MCLs in Folio, for example: Calendar settings -> Current calendar assignments and we believe there is the need for changes to Stripes to support MCLs that do not provide linkable row/row elements. If this is something we can fix in the application we will do so. If a Stripes level fix is required we will work with the Stripes team to work out the fix
  1. The dev team has been making active progress on the other two failing criteria (code smells and test coverage) and there is unanimous agreement on the solution and timeline. So they are pending as part of the provisional approval, see above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant