Skip to content
This repository was archived by the owner on Oct 10, 2024. It is now read-only.

Conversation

@Vyvy-vi
Copy link
Member

@Vyvy-vi Vyvy-vi commented Mar 20, 2021

closes #446

Description

This PR fixes the ^roles embed, which used to give assignment suggestion for non-assignable roles.
That might have been misleading to people.

What's the fix used?

A check to see if the role is in selfAssignableRoles.
(Could their be better ways of doing this rather than using .includes()?)

Screenshots

Before-
before image assignment error
After-
after adjustments

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

It's great having you contribute to this project

Welcome to the community 🤓

If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.

Copy link
Contributor

@AllanRegush AllanRegush left a comment

Choose a reason for hiding this comment

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

Looks Good 👍 Thank you. 😄

@AllanRegush
Copy link
Contributor

(Could there be better ways of doing this rather than using .includes()?)

In this case, it's okay. Since Assigned roles is an unsorted array. includes() is just a linear search. If the array was sorted we could use a bisection search. But the list isn't very long. So, it's not ideal.

The current Big O is O(1) if it's the first entry found or a Worst Case O(n) n = the length of the array.

@AllanRegush AllanRegush merged commit b3b6860 into EddieHubCommunity:develop Mar 22, 2021
@Vyvy-vi Vyvy-vi deleted the feat(446)/role-command-embed branch March 22, 2021 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the roles command

2 participants