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

Refactor {enable,disable}-hook-handler commands. #3310

Merged
merged 4 commits into from
Jan 17, 2024

Conversation

aadcg
Copy link
Member

@aadcg aadcg commented Jan 15, 2024

Description

These commands were suggested for deletion on #3297 but, on a closer look, I think they're in line with our goals.

I've revised the facilities.

I don't think it deserves a changelog entry.

Checklist:

  • Git branch state is mergable.
  • Changelog is up to date (via a separate commit).
  • New dependencies are accounted for.
  • Documentation is up to date.
  • Compilation and tests ((asdf:test-system :nyxt/gi-gtk))
    • No new compilation warnings.
    • Tests are sufficient.

@aadcg aadcg requested a review from jmercouris January 15, 2024 07:52
@aadcg aadcg added the 3-series Related to releases whose major version is 3. label Jan 15, 2024
@aadcg aadcg force-pushed the refactor-hook-handler-commands branch 2 times, most recently from 4f2e029 to f5a5853 Compare January 15, 2024 09:45
@aadcg aadcg mentioned this pull request Jan 15, 2024
19 tasks
@jmercouris
Copy link
Member

I'm not sure this is an improvement. Yes you are /technically/ removing code, but is there any real benefit? Also, do these /need/ to be commands? When would we ever use these?

@aadcg
Copy link
Member Author

aadcg commented Jan 16, 2024

We'll discuss it later today.

@jmercouris
Copy link
Member

OK!

@aadcg
Copy link
Member Author

aadcg commented Jan 17, 2024

We agreed that these commands are helpful to add/remove hook handlers at run time.

The goal of refactoring the commands isn't to remove code but to improve maintainability.

@aadcg aadcg force-pushed the refactor-hook-handler-commands branch from f5a5853 to 10303f5 Compare January 17, 2024 08:54
@aadcg aadcg merged commit f6701a2 into master Jan 17, 2024
2 checks passed
@aadcg aadcg deleted the refactor-hook-handler-commands branch January 17, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3-series Related to releases whose major version is 3.
Development

Successfully merging this pull request may close these issues.

2 participants