-
-
Notifications
You must be signed in to change notification settings - Fork 653
Issue#2853: Update once_event_filter in events.py #2858
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DeepC004 thanks for the PR, please add unit tests
@vfdev-5 thank you for reviewing the PR. I will make the changes as stated above and add unit tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @DeepC004 !
I left few other comments on how to improve this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @DeepC004 !
I started the CI. Once required tests are green we can merge the PR
@DeepC004 there are code formatting issues:
Please run the following locally to fix it: bash ./tests/run_code_style.sh install
bash ./tests/run_code_style.sh fmt However, the following errors should be fixed manually:
|
@DeepC004 please run |
I ran the code for formatting the files. The issue is length of some lines is greater than the maximum line length limit. It turns out to be a line for returning the error comment, which I believe can be written in more concise manner. |
Let me restart the CI by closing and reopening the PR |
@vfdev-5 thanks for helping me out throughout the PR. I will remember to consider small details like line length limit and junk whitespaces while contributing. This was my first merged PR, sorry incase I bugged you with doubts and code errors :D |
@DeepC004 thanks for your contribution and no worries about code formatting, this can be painful in the beginning ! Hope to see more of your contributions 👍 |
Related to #2853
Description:
Check list: