Conversation
Traditionally, the proper way to fix that is to rebase your branch and squash the commits. Do a web search to learn how to do that or watch this video: https://youtu.be/8j0H6urZ-bU?si=UinF0MCCK9QBRW0y. It's much tidier to do that than to close a PR and open a new PR. In practice you don't need to do that, however. GitHub added the capability to squash commits when merging to the web interface years ago, so project maintainers can take care of that. Some people still like to squash their commits themselves, of course. It looks cleaner, but it's usually unnecessary. Some projects still require contributors to squash their commits before merging (check the project's contributing guidelines if they have any), but that's kind of old fashioned. I would recommend renaming this pull request to remove the "as a single commit" part from the title of the PR. It's kind of confusing when dealing with transactional databases, and it's not meaningful in terms of the merge history, when/if it gets merged. |
|
Thanks for the feedback. But this is just supposed to be something that's easier to review as a whole, meaning, I'm planning to keep the full history for myself (which has already been rebased quite a bit to squash nuisance commits like fixes for odd typos). I'll change the name. |
…the async-prepare branch
|
@turnstep @rweikusat : Merging this PR caused the CI workflow to fail tests in |
|
The bigger deal is the test failure in https://github.com/bucardo/dbdpg/actions/runs/20106092843/job/57690271198#step:9:152 |
|
I can't promise a resolution today, but I'm looking into this. |
|
Hmm...CI should probably not be running the lint/critic/spellcheck tests, those are meant for authors only. Need to look into that. |
I don't see any reason not to run those tests. IMHO, I would keep that as-is. The annoying thing is that the CI workflow doesn't run automatically whenever a PR is opened. It's requiring manual approval. I think that can be changed, but I'm not sure. |
|
The pull request I just created (#152) should fix the pg_db_cancel issue. It doesn't address this 'failed to create dbh for sync connect test' issue. This works for me when running the test suite locallly. I realize this doesn't help much, ;-), but since it's 7:45pm here, I'll look into that tomorrow. |
|
Update on this: This is a test case I added which should do a synchronous connect when pg_async_connect => 0 was passed to the connect method. This works on my computer at home aka "the development machine." I'll see if I can reproduce this somehow via GitHub actions without abusing the original repository for that. So far, this fails because of a different "mystery error" and since I'm completely new to GitHub workflows, I have no idea how to fix this yet (or why it's happening). OTOH, since I don't really have anything to do at the moment (except job applications which seem entirely pointless) and was looking for a new technology project, anyway, I'll continue working on this although probably not before Monday. |
Support for asynchronous connect via PQconnectStart/ PQconnectPoll by passing a special attrbitute (pg_async_connect) to DBI::connect.
This is identical to the earlier async connect pull request but has all changes as single commit as my internal development history is probably not that interesting (or necessarily easy to follow).