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

Set expected priority sort order #1255

Closed
wants to merge 1 commit into from
Closed

Conversation

vilsbole
Copy link

The docs state that Higher priorities are favoured over lower priorities.
Set the correct sort order for priority.

If accepted I'm happy to setup and include tests.

The [docs](https://docs.ethers.io/v5/api/providers/other/#FallbackProviderConfig) state that `Higher priorities are favoured over lower priorities`.
Set the correct sort order for `priority`. 

If accepted I'm happy to setup and include tests.
@ricmoo
Copy link
Member

ricmoo commented Feb 1, 2021

Rather than change code which might affect users, I will update the docs. :)

@ricmoo ricmoo added documentation Documentation related issue. on-deck This Enhancement or Bug is currently being worked on. labels Feb 1, 2021
@ricmoo
Copy link
Member

ricmoo commented Feb 5, 2021

This has been fixed and published in the documentation. I see now the issue is that "higher priority" means a lower number (it can mean either, depending on the situation, but in my head that's how it works). So I've used the term "lower-value" and "higher-value" rather than the word priority to avoid any confusion at all.

Thanks! :)

@ricmoo ricmoo closed this Feb 5, 2021
@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation related issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants