8000 add a deprecated notice to 'SqlReader' for users by felixdivo · Pull Request #263 · hardbyte/python-can · GitHub
[go: up one dir, main page]

Skip to content

add a deprecated notice to 'SqlReader' for users#263

Closed
felixdivo wants to merge 6 commits intodevelopfrom
deprecate-notice
Closed

add a deprecated notice to 'SqlReader' for users#263
felixdivo wants to merge 6 commits intodevelopfrom
deprecate-notice

Conversation

@felixdivo
Copy link
Collaborator

I changed the structure of setup.py a little bit and added the Deprecated library as a dependency, as suggested by Brian. Then I added the @deprecated decorator to mark SqlReader as - you guessed it - deprecated.

@felixdivo felixdivo added this to the 2.2 Release milestone Feb 22, 2018
@felixdivo felixdivo self-assigned this Feb 22, 2018
@felixdivo felixdivo requested a review from hardbyte February 22, 2018 23:02
@felixdivo
Copy link
Collaborator Author

Oh wow, the Deprecated library is strange. Anyways, it seems like we forgot to include SqlReader in can/__init__.py and can/io/__init__.py, so it already broke comparability in the last Release. That makes all of this obsolete, right?

# TODO remove in later releases?
SqlReader = SqliteReader
# SqliteReader is the newer name
SqliteReader = SqlReader
Copy link
Owner

Choose a reason for hiding this comment

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

Won't the SqliteReader also be marked as deprecated with this approach?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes yes, it all doesn't work like I intended. But de we really need this deprecation at all? We already broke compatibility.

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm I didn't realize we'd already broken compatibility - in that case no we don't really need this deprecation warning. Lets just close this PR and remove the SqlReader/SqliteReader alias.

@felixdivo
Copy link
Collaborator Author

Yeah, I will do that.

@felixdivo felixdivo closed this in e4569c4 Feb 24, 2018
@felixdivo felixdivo deleted the deprecate-notice branch February 24, 2018 00:19
boris-wenzlaff pushed a commit to boris-wenzlaff/python-can that referenced this pull request Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0