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

Update setuptools #162

Merged
merged 7 commits into from
Jan 17, 2023
Merged

Update setuptools #162

merged 7 commits into from
Jan 17, 2023

Conversation

rrennick
Copy link
Collaborator

@rrennick rrennick commented Jan 16, 2023

See: #157

What

This PR

  • updates the build script to use setuptools build command instead of the deprecated setup.py script
  • adds an alternate Oauth config file path to mitigate a conflict with internal Tumblr tooling

Testing

  • Delete the dist and build folders if they exist in the root of the repository
  • Run python -m pip install --upgrade pip wheel build
  • Run python -m build
  • Run python interactive_console.py
  • Test various client methods from the README

@rrennick rrennick requested a review from sanmai January 16, 2023 19:02
@rrennick rrennick mentioned this pull request Jan 16, 2023
Copy link
Member

@sanmai sanmai left a comment

Choose a reason for hiding this comment

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

Code makes sense although I couldn't get it to run.

pyproject.toml Outdated
[project.urls]
"Homepage" = "https://github.com/tumblr/pytumblr"
"Bug Tracker" = "https://github.com/tumblr/pytumblr/issues"
"Download" = "https://github.com/tumblr/pytumblr/archive/0.1.1.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Download" = "https://github.com/tumblr/pytumblr/archive/0.1.1.tar.gz"

If we don't include this line we won't have to update it every time. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch. I changed it to link to the Releases page.

@@ -17,7 +17,7 @@ Install from source:

$ git clone https://github.com/tumblr/pytumblr.git
$ cd pytumblr
$ python setup.py install
$ python -m build
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to update .github/workflows/ci.yaml as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The PR build wasn't working for you because the testing steps didn't include installing build. I updated the instructions and ci.yaml to include installing and using build.

@rrennick rrennick merged commit d413e92 into master Jan 17, 2023
@rrennick rrennick deleted the wpmuguru/setuptools branch January 17, 2023 17:41
@rrennick rrennick mentioned this pull request Jan 17, 2023
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.

2 participants