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

fix(aria/get-role-type): work with standards object #2361

Merged
merged 1 commit into from
Jul 13, 2020
Merged

Conversation

straker
Copy link
Contributor

@straker straker commented Jul 10, 2020

Refactored get-role-type to have a better happy path and return early rather than doing the && and || checks.

Part of #2108

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Code is reviewed for security

@straker straker requested a review from a team as a code owner July 10, 2020 16:33
@straker straker changed the title fix(aria/get-role-type): workd with standards object fix(aria/get-role-type): work with standards object Jul 10, 2020
return null;
}

return roleDef.type;
Copy link
Member

@stephenmathieson stephenmathieson Jul 12, 2020

Choose a reason for hiding this comment

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

Previously, this would return null if there wasn't a truthy .type. Do we need to maintain consistently, or is the possibility of returning undefined here ok?

Copy link
Contributor Author

@straker straker Jul 13, 2020

Choose a reason for hiding this comment

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

Every role should have a type so it shouldn't be possible to return undefined or null. A role without a type should be treated as a bug.

@straker straker merged commit a61e314 into develop Jul 13, 2020
@straker straker deleted the get-role-type branch July 13, 2020 14:50
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.

3 participants