bugen / pypipe

Python pipe command line tool
Apache License 2.0
796 stars 25 forks source link

feat: add tests for ppp_line and ppp_res examples #12

Closed horw closed 1 year ago

horw commented 1 year ago

Add base tests for examples from README.md. close #11

horw commented 1 year ago

@bugen, I have just added a few 'lines' and 'record' tests. Could you please take a look and leave your comments? My plan is to add all the examples from the README.md

bugen commented 1 year ago

@horw Cool! Very clear, versatile. Is it better to merge at this point, or should I wait until additional test cases are added?

horw commented 1 year ago

@horw Cool! Very clear, versatile. Is it better to merge at this point, or should I wait until additional test cases are added?

It's nice to hear that you like it. Just wait for the rest of the examples; I will complete them in few days.

horw commented 1 year ago

@bugen you can check for updates. I've already added most of the tests and have also included pre-commit and CI tests.

bugen commented 1 year ago

Thank you @horw . The test code is excellent. I also appreciate the information about pre-commit and ci.yml.

Could you please explain ci.yml? Am I correct in understanding that ruff only performs linting and it doesn't handle formatting? If there are some linting errors, does that mean push won't be allowed?

horw commented 1 year ago

This basic CI.yml configuration for linting and tests without formatting. Its main purpose is to ensure that when people create new pull requests for your project, you can quickly determine if they pass basic tests. Sometimes, making changes might unexpected break other parts of the project. You can set up more complete tests using GitHub Actions, btw those are separate from the current pull request's purpose.

Pre-commit is also very helpful for maintaining code quality, but it requires installation to enforce these rules. For the next PR you can add additional rules for contributing.

For further reading on this topic:

bugen commented 1 year ago

@horw Thank you for the explanation. I have a general understanding of what you are trying to do. Maintaining code quality is important, so I agree with the content of this PR. When I checked the current pypipe.py using ruff, I already found some errors... Is it okay to merge this content for now, or are there still additional changes to be made to this PR?

horw commented 1 year ago

P.S. there are many other linters available such as flake 8, black, etc. And you can choose which you like. Yes, you can merge the current content.

bugen commented 1 year ago

@horw I merged. Thank you for your contribution.