Skip to content
New issue

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

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

Already on GitHub? # to your account

docs: reorder README sections in order of importance #140

Merged
merged 1 commit into from
Oct 29, 2022

Conversation

empicano
Copy link
Owner

Hi Frederik!

Again with the README 😄 I tweaked a few little things, and reordered the sections, among others to move the Note for Windows users higher, as there are quite a few people who seem to overlook this. (Btw. is there anything that speaks against implementing the switch to the SelectorEventLoop directly in the code? I guess changing the loop silently is not a very nice pattern.)

~ Felix

@JonathanPlasse
Copy link
Collaborator

Why delete the requirement section?

@empicano
Copy link
Owner Author

I didn't delete it, I just merged it with the installation section 😉

@frederikaalund
Copy link
Collaborator

Good idea to move the "Note for Windows users" section up. 👍 We get a frequent stream of new issues due to that.

(Btw. is there anything that speaks against implementing the switch to the SelectorEventLoop directly in the code? I guess changing the loop silently is not a very nice pattern.)

Technically, there is no barrier. :) It's just intrusive to switch the loop like that IMHO. What we could do, is to issue a warning if the user attempts to use the library with a non-supported event loop. In that warning, we can instruct the user to switch the event loop. I weldcome a PR with that change. 👍

Back to this PR itself. LGTM. Thanks!

~Frederik

@frederikaalund frederikaalund merged commit 7ab7cc6 into master Oct 29, 2022
@empicano empicano deleted the reorder-readme branch October 29, 2022 13:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants