Skip to content

Conversation

@eigenein
Copy link
Collaborator

@eigenein eigenein commented Oct 2, 2018

  • Add tox.ini, support 2.6, 2.7, 3.4, 3.5, 3.6 and 3.7
  • Drop Python 3.2 and 3.3 support
  • Improve PEP8-compliance in a few places
  • Bump version to 1.0.2 and mark it as stable
  • Add classifiers to setup.py
  • Improve .gitignore with standard templates for popular environments
  • Remove some dead code in comments

@eigenein
Copy link
Collaborator Author

@podshumok Could you please take a quick look if it's okay for you?

if sequence_generator is None:
sequence_generator = SimpleSequenceGenerator()
self.sequence_generator = sequence_generator
self.sequence_generator = sequence_generator or SimpleSequenceGenerator()
Copy link
Member

Choose a reason for hiding this comment

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

I think is None version is better here as people may want to override __bool__ for things like sequence generators

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, makes sense. I'll change it back.

@code-of-kpp
Copy link
Member

Some strings are too long for my screen (I have vision problems and hence I use PEP8 recommended 79 columns). Apart from that it looks good.

@eigenein
Copy link
Collaborator Author

Got it, will reformat those lines.

* Add `tox.ini`, support `2.6`, `2.7`, `3.4`, `3.5`, `3.6` and `3.7`
* Drop Python `3.2` and `3.3` support
* Improve PEP8-compliance in a few places
* Bump version to `1.0.2` and mark it as stable
* Add classifiers to `setup.py`
* Improve `.gitignore` with standard templates for popular environments
* Remove some dead code in comments
@eigenein
Copy link
Collaborator Author

Oki, addressed the comments.

@eigenein eigenein merged commit 9850f8e into python-smpplib:master Oct 11, 2018
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