Skip to content

project structure and packaging revamp#188

Merged
aoberoi merged 4 commits intoslackapi:masterfrom
aoberoi:structure-fixes
Jun 23, 2017
Merged

project structure and packaging revamp#188
aoberoi merged 4 commits intoslackapi:masterfrom
aoberoi:structure-fixes

Conversation

@aoberoi
Copy link
Contributor

@aoberoi aoberoi commented Jun 21, 2017

  • deleted top level __init__.py - its not part of the default code structure

  • improved setup.py:

    • DRYed up install_requires by allowing requirements.txt to just read from here
    • gave dependencies version ranges
    • added long_description property
    • changed url property to an https one
    • changed author and author_email property to reflect the entire company
    • added classifiers property
    • added keywords property
    • remove zip_safe property

  • specified flake8 configuration in .flake8

  • added support for py33 (its not EOL until sept 2017)

  • modified travis build

    • use travis' own virtualenv switching system instead of relying on tox to do it
    • fail earlier if lint does not succeed
    • moved coverage tool configuration to .coveragerc

  • enabled binary distribution as a universal wheel (python2 and python3)

  • removed unncessary test dependencies

  • simplified and commented tox.ini

    • implemented docs generation as a tox task

  • remove single underscore for all filenames inside slackclient/ (except __init__.py)

  • change slackclient/__init__.py to from .client import SlackClient

  • badges for python version support

  • get CI for windows (AppVeyor)

  • added new oss-guidelines

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.

  • I've read and agree to the Code of Conduct.

  • I've been mindful about doing atomic commits, adding documentation to my changes, not refactoring too much.

  • I've a descriptive title and added any useful information for the reviewer. Where appropriate, I've attached a screenshot and/or screencast (gif preferably).

  • I've written tests to cover the new code and functionality included in this PR.

  • I've read, agree to, and signed the Contributor License Agreement (CLA).

@aoberoi
project structure and packaging revamp
395faff 
*  deleted top level `__init__.py` - its not part of the default code structure
*  improved `setup.py`:
   -  DRYed up `install_requires` by allowing `requirements.txt` to just read from here
   -  gave dependencies version ranges
   -  added `long_description` property
   -  changed `url` property to an https one
   -  changed `author` and `author_email` property to reflect the entire company
   -  added `classifiers` property
   -  added `keywords` property
   -  remove `zip_safe` property
*  specified `flake8` configuration in `.flake8`
*  added support for py33 (its not EOL until sept 2017)
*  modified travis build
   -  use travis' own virtualenv switching system instead of relying on tox to do it
   -  fail earlier if lint does not succeed
   -  moved `coverage` tool configuration to `.coveragerc`
*  enabled binary distribution as a universal wheel (python2 and python3)
*  removed unncessary test dependencies
*  simplified and commented `tox.ini`
   -  implemented docs generation as a tox task
*  remove single underscore for all filenames inside `slackclient/` (except `__init__.py`)
*  change `slackclient/__init__.py` to `from .client import SlackClient`
*  badges for python version support
*  get CI for windows (AppVeyor)
*  added new oss-guidelines
Roach
Roach approved these changes Jun 21, 2017
View reviewed changes
Copy link
Contributor

@Roach Roach left a comment

Choose a reason for hiding this comment

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

🙌 Awesome work here, this module has been needing some cleanup

@aoberoi
Copy link
Contributor Author

aoberoi commented Jun 21, 2017 •
edited
Loading

3 things to address before we merge:

  • update the HTML docs' instructions on building the HTML docs (meta!)
  • get Windows CI to build successfully
  • DISCUSS is it acceptable to rename the submodules (e.g. slackclient._slackrequest ▶️ slackclient.slackrequest) without introducing a major version change?

To kick off the discussion on the versioning:

the reason for renaming them in the first place is to adhere with module naming best practices:

If you’d like you could name your module my_spam.py, but even our friend the underscore should not be seen often in module names. Hitchikers Guide to Python

the rationale for not introducing this in a major version is that the submodules were previously "internal" bits of the SDK so no external users should depend on them, and as such this shouldn't break anyone's code.

i think we need a few more opinions here, so i'll loop in some folks (@Roach, @karishay, @Yasumoto , @rawdigits, @harrisonpage, @rickhanlonii)

@aoberoi
Copy link
Contributor Author

aoberoi commented Jun 21, 2017 •
edited
Loading

here's an alternative that might be worth implementing: https://stackoverflow.com/a/3645219/305340

the recommendation is to create a submodule called private in which all the private modules reside, so that its as explicit as possible that those modules are not meant to be used.

@codecov
Copy link

codecov bot commented Jun 21, 2017 •
edited
Loading

Codecov Report

Merging #188 into master will decrease coverage by 4.6%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
- Coverage   66.44%   61.84%   -4.61%     
==========================================
  Files           9        9              
  Lines         304      304              
  Branches        0       56      +56     
==========================================
- Hits          202      188      -14     
  Misses        102      102              
- Partials        0       14      +14
Impacted Files Coverage Δ
slackclient/util.py 80.95% <ø> (ø)
slackclient/im.py 0% <ø> (ø)
slackclient/channel.py 80.76% <ø> (ø)
slackclient/slackrequest.py 100% <ø> (ø)
slackclient/user.py 66.66% <ø> (ø)
slackclient/client.py 38.88% <100%> (ø)
slackclient/server.py 61.86% <100%> (ø)
slackclient/__init__.py 100% <100%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8ebd4a...02eb3f2. Read the comment docs.

@aoberoi aoberoi mentioned this pull request Jun 22, 2017
Closed
3 tasks
@aoberoi aoberoi merged commit d95ea21 into slackapi:master Jun 23, 2017
c-goosen pushed a commit to c-goosen/python-slackclient that referenced this pull request Jun 18, 2019
Merge pull request slackapi#188 from aoberoi/structure-fixes
e2c1b30 
project structure and packaging revamp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Roach Roach Roach approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aoberoi @Roach