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

Deprecation warnings #23

Merged
merged 2 commits into from
Dec 6, 2015
Merged

Deprecation warnings #23

merged 2 commits into from
Dec 6, 2015

Conversation

eisensheng
Copy link
Owner

This adds a longer paragraph regarding the deprecation phase for the changed methods. The CHANGES.rst file will also be included in the long_description for the setup.py file to warn users only reading the PYPI page.

Would be nice to have this reviewed before it lands in develop. English is not my mother language after all. I hate to state this but this makes it a requirement that other review my text IMHO. Thanks!


- ``caplog.atLevel`` for ``caplog.at_level`` and
``caplog.setLevel`` for ``caplog.setLevel`` are still available
but not supported and deprecated since they don't follow the
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess, deprecated and not supported would be less ambiguous. :)

@abusalimov
Copy link
Collaborator

I'm not a lawyer native either, but it looks good to me except for few minor notes above.

@@ -12,6 +12,22 @@ Yet to be released.

- [Bugfix] #15 #17 - Restore Python 2.6 compatibility. (Thanks to Marco Nenciarini!)

.. attention::
Deprecation warnings, these symbols are slated for removal in
the next major release.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sentence reads weird IMHO (it starting with "Deprecation warnings, ..."). I also don't think "symbols" is a common term for Python.

What about: "Deprecation warning: These functions and attributes are slated for ..."?

@eisensheng
Copy link
Owner Author

I processed your feedback and reworked the wording a bit. I would appreciate it if someone could read over it again so I can merge this together with the other Pull Requests and Release 1.2.1 from it. Thanks! 😎

@@ -12,6 +12,23 @@ Yet to be released.

- [Bugfix] #15 #17 - Restore Python 2.6 compatibility. (Thanks to Marco Nenciarini!)

.. attention::
Deprecation warnings, these objects (i.e. functions, properties) are
slated for removal in the next major release.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still think starting a sentence with "Deprecation warnings, these objects ..." makes no sense 😉

I think "Deprecation warning: These objects ..." would be better.

@The-Compiler
Copy link
Collaborator

LGTM apart from the comment above.

I think #24/#25/#26 still need some more testing though. No idea in what state the other PRs are right now.

@abusalimov
Copy link
Collaborator

No-no-no, these must not go into v1.2.1.
I'll add more tests there once I finish preparing the performance changeset I mentioned in #26.

@eisensheng
Copy link
Owner Author

Hopefully I got it right this time.

@abusalimov
Copy link
Collaborator

I think, it's worth squashing your follow-up changes into the first commit.

@eisensheng eisensheng force-pushed the deprecation-warnings branch from 06e43e5 to da38a80 Compare December 6, 2015 22:43
eisensheng added a commit that referenced this pull request Dec 6, 2015
@eisensheng eisensheng merged commit 8ded66e into develop Dec 6, 2015
@abusalimov abusalimov deleted the deprecation-warnings branch December 7, 2015 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants