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

Python38 and refactor tests to use pytest-asyncio #18

Merged
merged 14 commits into from
Mar 9, 2021
Merged

Conversation

FlorianLudwig
Copy link
Member

rebased version of njouanin/hbmqtt#226

depends on #17

@FlorianLudwig FlorianLudwig marked this pull request as draft March 6, 2021 17:47
@FlorianLudwig FlorianLudwig changed the title Python38 Python38 and refactor tests to use pytest-asyncio Mar 6, 2021
@FlorianLudwig FlorianLudwig self-assigned this Mar 6, 2021
@FlorianLudwig FlorianLudwig marked this pull request as ready for review March 6, 2021 18:03
@HerrMuellerluedenscheid
Copy link
Contributor

I love where this is going! Thanks for addressing the tests @FlorianLudwig!

@HerrMuellerluedenscheid HerrMuellerluedenscheid linked an issue Mar 7, 2021 that may be closed by this pull request
@HerrMuellerluedenscheid
Copy link
Contributor

@FlorianLudwig do we want to squash commits? I could squash all of mine into a single python3 refactor I guess. But I don't want to do that without coordinating with you to avoid conflicts. If you consider squashing any of your feel free to do the same for mine. But after all it's just cosmetic. So, what do you think about squashing?

@FlorianLudwig FlorianLudwig removed their assignment Mar 8, 2021
@FlorianLudwig
Copy link
Member Author

tl;dr

pragmatic, i would just leave it as is.

squashing commits

It is on my TODO list to write my thoughts about squashing - a complicated topic indeed. So let me have a take on it :) Just to make a simple question a discussion about principles.

The git history is part of the work of a developer and not a backup. It should tell a story, each commit is one scene. Jumping to much between them and you lose your reader (that might be yourself in a few month, trying to understand why the code ended up the way it did or some psychopath).

Some rules of thumb:

  • One commits should not never include unrelated changes

  • Generally, try to keep commits small, making it easier to review each commit on it's own

  • Automated changes to files that do not change the meaning (e.g. reformatting) must be a separate commit with the tools used clearly stated in the commit message

  • Often before doing the change you actually want to do, there is some refactoring involved, in that case:

    Move refactoring into its own commit.
    A refactoring should keep the same end result of the software, while changing the code.
    Having a refactoring separated into a separate commit from other changes keeps them comprehensible and reviewable.

@FlorianLudwig FlorianLudwig deleted the python38 branch March 21, 2021 15:47
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.

Implement tests based on pytest
2 participants