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

adodbapi: Cleanup obsolete and unsupported python code #2094

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Aug 6, 2023

Following a general understanding that adodbapi code changes will be a lot harder to test, and comments in #1990, these changes have been completely split off. With the final goal to make basic type-checking validation of public methods possible, easing the addition of 3.7+ type annotations. I am fine if that excludes adodbapi.

A handful of PRs listed below have overlapping or extracted changes. I'd recommend reviewing those first, in the mean time I'm marking this PR as draft.

This cleans-up most unreachable code. Mostly due to unsupported python versions.

adodbapi/remote/server.py Outdated Show resolved Hide resolved
@vernondcole
Copy link
Collaborator

vernondcole commented Aug 9, 2023 via email

@vernondcole
Copy link
Collaborator

vernondcole commented Aug 9, 2023 via email

@Avasam Avasam changed the title Cleanup obsolete and unsupported python code in adodbapi adodbapi: Cleanup obsolete and unsupported python code Aug 12, 2023
@Avasam Avasam force-pushed the remove-adodbapi-only-obsolete-python-code branch from fef1f02 to 4ba4e43 Compare March 17, 2024 03:29
Copy link
Collaborator

@vernondcole vernondcole left a comment

Choose a reason for hiding this comment

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

Is this just to pull upstream changes into the adodbapi fork?

@Avasam
Copy link
Collaborator Author

Avasam commented Mar 28, 2024

@vernondcole This is a "too big to review" PR that has been split up into smaller chunks in other PRs. It contains all of the code modernization and old code removal changes at once. Useful for me to reference when there will undoubtedly be merge conflicts, and allows me to test the interaction of different changes in their final combined state.

Hence it's marked as Draft. Once everything else is merged I'll either Publish this PR or keep splitting off what's left in more reviewable chunks.

As per the PR description:

A handful of PRs listed below have overlapping or extracted changes. I'd recommend reviewing those first, in the mean time I'm marking this PR as draft.

@vernondcole
Copy link
Collaborator

Understood.
I am still working on creating a set of VMs so that I can properly test this and get everything merged back in.

adodbapi/readme.txt Outdated Show resolved Hide resolved
@Avasam Avasam marked this pull request as ready for review April 24, 2024 20:34
@Avasam Avasam force-pushed the remove-adodbapi-only-obsolete-python-code branch from dcea067 to 54ebec1 Compare April 24, 2024 20:53
@Avasam Avasam requested a review from vernondcole April 24, 2024 20:53
@Avasam
Copy link
Collaborator Author

Avasam commented Apr 24, 2024

@vernondcole This is now in a state ready to be reviewed. The bulk has been extracted in other PRs.

@Avasam Avasam force-pushed the remove-adodbapi-only-obsolete-python-code branch 3 times, most recently from ec9bd31 to d91f3a1 Compare April 24, 2024 21:22
@Avasam Avasam mentioned this pull request Jul 22, 2024
@Avasam Avasam force-pushed the remove-adodbapi-only-obsolete-python-code branch 2 times, most recently from 33ad767 to 086a93d Compare October 4, 2024 21:12
@Avasam Avasam force-pushed the remove-adodbapi-only-obsolete-python-code branch from 086a93d to 2706c93 Compare October 4, 2024 21:13
@Avasam
Copy link
Collaborator Author

Avasam commented Oct 18, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants