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

Add non-py files watch support #14

Merged
merged 4 commits into from Jun 2, 2023

Conversation

aptakhin
Copy link
Contributor

Hello!

Thank you very much for the simple and working pytest-watcher solution!

I've missed a little functionality about filtering watch files above *.py. Because in my projects also .env and .ini files can widely change the behaviour.

I added arguments to the command line now. In my case, it is okay as it. In future, we can also put some of these --include-filter and --ignore-filter arguments to the configuration file.

And I also have small doubts about confusing examples. But for now, I have no idea how to make it more clear except documentation remark. master...aptakhin:pytest-watcher:add-other-extensions-support#diff-58fb153a128d801a5a58c94d9fcd912af6349faa66d202cc43ec4e1c23ae25dfR54-R62

@olzhasar olzhasar added the enhancement New feature or request label Jan 4, 2023
@aptakhin
Copy link
Contributor Author

Hello @olzhasar,

Do you have any comments here about argument naming or idea?
Should I continue to polish the final things?

@olzhasar
Copy link
Owner

olzhasar commented Feb 4, 2023

Hey @aptakhin,

Sorry for the delay, didn't have much time lately.
Thank you for your contribution!

I generally like the proposed suggestion, but I have some concerns about naming and the code.
I will take some time to think about it and possibly come up with some ideas afterwards

Copy link
Owner

@olzhasar olzhasar left a comment

Choose a reason for hiding this comment

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

Hi @aptakhin ,

Apologies for the extremely long delay in my response. I haven't had much time for maintenance lately

I've added some review comments here. Let me know if you agree / disagree with them. In case you don't have time to make changes to this PR, I can update it on my own later.

pytest_watcher/watcher.py Outdated Show resolved Hide resolved
pytest_watcher/watcher.py Outdated Show resolved Hide resolved
pytest_watcher/watcher.py Outdated Show resolved Hide resolved
include_filter -> patterns
ignore_filter -> ignore_patterns
@olzhasar
Copy link
Owner

olzhasar commented Jun 2, 2023

Hey @aptakhin,

I've taken a bit different approach for the implementation - basically moving all the pattern matching logic to the existing EventHandler class instead of creating a new class.
Even though the code differs almost entirely from the original PR version, I've made all the changes here to preserve your contribution.

Thanks once again!

@olzhasar olzhasar merged commit 164c089 into olzhasar:master Jun 2, 2023
@aptakhin
Copy link
Contributor Author

aptakhin commented Jun 6, 2023

Hi @olzhasar,
I'm sorry, I also got busy without answering things. Thank you very much for the help with the solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants