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

Roadmap for 0.99 #306

Closed
sigmavirus24 opened this issue Nov 10, 2014 · 2 comments

Comments

@sigmavirus24
Copy link
Owner

commented Nov 10, 2014

As it stands now, 1.0 just has a few items left before it can ship.

I would love to have someone help getting 0.99 out the door. This release is as important as 1.0.

What is necessary for this release is for the following checklist to be applied to most of the breaking changes in 1.0:

  • Add back the definition for a method that has been removed
  • Add a deprecation warning
  • Ensure the definition, simply calls the method in 1.0
  • Add a unit test that checks for the calls.

Take for example, github3.iter_all_repos:

# github3/api.py
def iter_all_repos(
        # params
        ):
    warnings.warn('iter_all_repos will be removed in v1.0. You can switch to using all_repositories now.',
                  DeprecationWarning)
    return all_repositories(
        # params
    )

A slightly more complex would be GitHub#iter_gists.

# github3/github.py
class GitHub(...):
    def iter_gists(self,
                   # params
                   ):
        if username is not None:
            warnings.warn(
                "Retrieving the a user's gists will be its own method in 1.0."
                "The new method is :func:`~GitHub.gists_by` and can be used immediately.",
                DeprecationWarning
                )
            return self.gists_by(...)
        if authenticated:
            warnings.warn(
                "Retrieving the authenticated user's gists will be its own method in 1.0."
                "The new method is :func:`~GitHub.gists` and can be used immediately.",
                DeprecationWarning
                )
            return self.gists(...)
        if fetch_all:
            warnings.warn(
                "Retrieving the all public gists will be its own method in 1.0."
                "The new method is :func:`~GitHub.public_gists` and can be used immediately.",
                DeprecationWarning
                )
            return self.public_gists(...)

Keep in mind these are just pseudo-examples. Because we will be using warnings.warn so much, a convenience/utility function such as issue_warning that might look like:

def issue_warning(message, deprecation_class=DeprecationWarning):
     warnings.warn(message, deprecation_class)

Finally, let's look at some example tests for this

# tests/unit/test_api_compat.py

def test_iter_all_repos_calls_all_repositories(...):
    """Verify that iter_all_repos proxies to all_repositories."""
    with mock.patch('github3.api.all_repositories') as all_repositories:
        github3.api.iter_all_repos(...)
        assert all_repositories.assert_called_once_with(...)
# tests/unit/test_github_compat.py

class TestGitHubCompat(...):
    def setUp(self):
        self.gh = github3.github.GitHub()

    def test_iter_gists_calls_gists(...):
        """Verify that iter_gists proxies to gists when authenticated."""
        self.gh.login(...)
        with mock.patch.object(self.gh, 'gists') as gists:
            self.gh.iter_gists(...)
            gists.assert_called_once_with(...)

    def test_iter_gists_calls_gists_by(...):
        """Verify that iter_gists proxies to gists_by when given a username."""
        with mock.patch.object(self.gh, 'gists_by') as gists_by:
            self.gh.iter_gists('username', ...)
            gists_by.assert_called_once_with('username', ...)

    def test_iter_gists_calls_public_gists(...):
        """Verify that iter_gists proxies to public_gists without auth or username."""
        with mock.patch.object(self.gh, 'public_gists') as public_gists:
            self.gh.iter_gists(...)
            public_gists.assert_called_once_with(...)


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@jacobian

This comment has been minimized.

Copy link

commented Nov 10, 2014

@sigmavirus24 you may be interested in stealing some code from Django: django/utils/deprecation.py contains a warn_about_renamed_method decorator that we've used in the past when we need to move methods around. It might make this sort of rearranging easier.

@sigmavirus24

This comment has been minimized.

Copy link
Owner Author

commented Nov 10, 2014

❤️ @jacobian that looks awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.