-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
supplement Postgres listen example with a small chat example #2577
supplement Postgres listen example with a small chat example #2577
Conversation
6fa7913
to
ed8bab0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also adjust the Github Actions workflow that tests this example?
You can just update the entry for the listen example: https://github.com/launchbadge/sqlx/blob/main/.github/workflows/examples.yml#L184-L196
Those entries are alphabetized to make it easier to navigate that file, so this should also be moved above Files (Setup)
to keep things organized.
ed8bab0
to
0553981
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is failing CI because it's expecting a TTY and not getting one, which is a real issue for automated testing. You'd have to specially allocate a TTY for the process, I think. But even so, it'd just sit there forever unless you actually sent it commands.
If you want to figure out how to automate TUI testing in CI, that's your prerogative, but I think it's good enough if we just check that it compiles. That should be noted in the README in case someone tries it and has problems with it.
That does make me hesitant to delete the listen
example, however, as that was able to run in CI just fine. There's no reason not to have both, I think, since it's all handled in the same CI job anyway.
0553981
to
d47531e
Compare
Yeah rather not go for the testing in CI for this. |
Yeah just keeping the listen example is fine @JockeM, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One remaining nit + undeleting listen example.
Co-authored-by: Stephen <[email protected]>
d47531e
to
0b55ac5
Compare
Sorry for missing the request on the original PR! It slipped through on my end. Glad to see the changes were made and this was merged! (: |
Closes #161
this is just a continuation of #2165 made by Stephen
i just took his chat implementation and fixed the readme and renamed the folder to chat