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

filter: Remove obsolete function add_trigger #1807

Merged

Conversation

sypark9646
Copy link
Contributor

@sypark9646 sypark9646 commented Aug 25, 2023

The function add_trigger has been replaced with update_trigger,
rendering it unnecessary. This commit removes the now-unused
add_trigger function.

related: eeb5d74

Signed-off-by: Soyeon Park sypark9646@gmail.com

@sypark9646 sypark9646 force-pushed the refactor/remove-add-trigger-function branch 2 times, most recently from 7d25b04 to 201d429 Compare August 25, 2023 07:36
@sypark9646 sypark9646 marked this pull request as draft August 25, 2023 07:45
@sypark9646 sypark9646 changed the title refactor: remove changed function: add_trigger -> update_trigger refactor: Remove obsolete function add_trigger Aug 25, 2023
@sypark9646 sypark9646 force-pushed the refactor/remove-add-trigger-function branch from 201d429 to 3b985f4 Compare August 25, 2023 08:06
@sypark9646 sypark9646 marked this pull request as ready for review August 25, 2023 08:10
Copy link
Collaborator

@honggyukim honggyukim left a comment

Choose a reason for hiding this comment

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

The change looks good, but we don't use refactor: as a prefix in the commit title. You can get more idea by reading other commit messages by running git log.

In this case, the prefix could be filter: because it modifies the filter.h.

@sypark9646 sypark9646 changed the title refactor: Remove obsolete function add_trigger filter: Remove obsolete function add_trigger Aug 26, 2023
@sypark9646 sypark9646 closed this Aug 26, 2023
@sypark9646 sypark9646 deleted the refactor/remove-add-trigger-function branch August 26, 2023 10:42
@sypark9646
Copy link
Contributor Author

@honggyukim Thank you! I'll fix the branch naming and PR again

@honggyukim
Copy link
Collaborator

No, please reopen this PR and force push to the same branch to update this patch.

@honggyukim
Copy link
Collaborator

I will reopen this PR.

@honggyukim
Copy link
Collaborator

Hmm.. the branch is already deleted.

@sypark9646 sypark9646 restored the refactor/remove-add-trigger-function branch August 26, 2023 11:04
@sypark9646
Copy link
Contributor Author

Sorry, when I renamed the branch in the forked repository, the old branch disappeared - I've restored it now!

@sypark9646 sypark9646 reopened this Aug 26, 2023
The function add_trigger has been replaced with update_trigger,
rendering it unnecessary. This commit removes the now-unused
add_trigger function.

Signed-off-by: Soyeon Park <sypark9646@gmail.com>
@sypark9646 sypark9646 force-pushed the refactor/remove-add-trigger-function branch from 3b985f4 to 58e37d6 Compare August 26, 2023 11:51
Copy link
Collaborator

@honggyukim honggyukim left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix!

@honggyukim honggyukim merged commit d6f6edb into namhyung:master Aug 26, 2023
3 checks passed
@sypark9646 sypark9646 deleted the refactor/remove-add-trigger-function branch August 28, 2023 03:29
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.

None yet

2 participants