Skip to content

Single-sourcing the package version, user-agent and bug fixes #24

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

Merged
merged 7 commits into from
Feb 26, 2021

Conversation

fatihkurtoglu
Copy link
Contributor

Summary of changes in separate commits as follows:

  • Fixed project methods (naming typo in create_project and missing payload in update_project)
  • Removed build folder from the repo. It's only required while building locally, similar to dist folder
  • Enabled User-Agent parameter in request headers with module version, Python version and OS details. Sample:
scaleapi/1.0.2 Python/3.6.12 OS/Darwin-19.6.0-x86_64-i386-64bit
  • Moved package versioning to a separate file: _version.py according to common practices:
    • _version.py will have a line for version in __version__ variable
    • Same variable will be available within module as scaleapi.__version__ as in most packages
    • Same value will be used in setup.py file
    • This approach is built according to one of the recommended practices in Python Packaging User Guide

Version bumped to 1.0.2 in file but will be built and uploaded to pypi after merge.

Copy link
Contributor

@shaun-scale shaun-scale left a comment

Choose a reason for hiding this comment

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

Amazing upgrades here @fatihkurtoglu - I had already bumped to v1.0.2 yesterday (sorry I wasn't clear there) and then I had one comment about our versioning logic but all the important stuff is looking great.

setup.py Outdated
Comment on lines 39 to 40
else:
raise RuntimeError("Unable to find a valid __version__ string in %s." % rel_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a cool python trick to put else: after a for loop or was the tab indentation just off here? I suppose we could just have it be raise without the else: to the same effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, for/else is a pythonic way. else statement is triggered when the loop ends until the end without any interruption (break/return). More like a then stage for the loop.

Yes, it's the same as just keeping the raise there as you said, I'll update it.

Copy link
Contributor Author

@fatihkurtoglu fatihkurtoglu left a comment

Choose a reason for hiding this comment

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

Version upgraded.

@fatihkurtoglu
Copy link
Contributor Author

@shaun-scale Made the changes regarding version bump and for/else statement.

@shaun-scale shaun-scale self-requested a review February 26, 2021 18:42
Copy link
Contributor

@shaun-scale shaun-scale left a comment

Choose a reason for hiding this comment

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

Looks great

@fatihkurtoglu fatihkurtoglu merged commit ada216a into master Feb 26, 2021
@fatihkurtoglu fatihkurtoglu deleted the useragent-and-fixes branch February 26, 2021 18:58
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