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.

Sign up for GitHub

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

Jump to bottom

[Refactor] qmk find #21096

Merged
merged 7 commits into from
Feb 17, 2024
Merged

[Refactor] qmk find #21096

merged 7 commits into from
Feb 17, 2024

Conversation

elpekenin
Copy link
Contributor

Description

Followup to #21090, move from if/else into a func_name: lambda mapping and use partial to capture the values.
This reduces some duplication and makes it a little easier to add a new filter.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added cli qmk cli command python labels May 31, 2023
@elpekenin elpekenin changed the title Branch point for 2023q3 Breaking Change. [Refactor] qmk find May 31, 2023
sigprof
sigprof requested changes Jun 1, 2023
View reviewed changes
lib/python/qmk/search.py Outdated Show resolved Hide resolved
lib/python/qmk/search.py Outdated Show resolved Hide resolved
@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Sep 19, 2023
@tzarc tzarc requested review from a team and sigprof September 19, 2023 23:43
@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Sep 20, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 5, 2023

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Nov 5, 2023
@tzarc tzarc added breaking_change_2024q1 and removed breaking_change_2023q4 stale Issues or pull requests that have become inactive without resolution. labels Nov 26, 2023
@elpekenin elpekenin force-pushed the find_mapping branch 2 times, most recently from 15f8d93 to ec8e6d1 Compare November 28, 2023 15:05
@github-actions github-actions bot added core keyboard keymap documentation via Adds via keymap and/or updates keyboard for via support labels Nov 28, 2023
@github-actions github-actions bot removed the via Adds via keymap and/or updates keyboard for via support label Nov 28, 2023
@elpekenin elpekenin requested a review from tzarc November 28, 2023 15:34
@elpekenin
Copy link
Contributor Author

Changed the code substantially, existing review does not apply anymore

elpekenin
elpekenin commented Nov 28, 2023
View reviewed changes
lib/python/qmk/search.py
continue

cli.log.info(f'Filtering on condition: {{fg_green}}{func_name}{{fg_reset}}({{fg_cyan}}{key}{{fg_reset}})...')
filter_class = _get_filter_class(func_name, key, value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is no longer checking that the binary (key and value) filters receive value != None. Shall perhaps add it back

@tzarc tzarc requested a review from a team February 16, 2024 13:38
@zvecr zvecr merged commit 6810aaf into qmk:develop Feb 17, 2024
4 of 5 checks passed
@elpekenin elpekenin deleted the find_mapping branch February 17, 2024 12:46
nuess0r pushed a commit to nuess0r/qmk_firmware that referenced this pull request Sep 8, 2024
@elpekenin @nuess0r
[Refactor] qmk find (qmk#21096)
6a150f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzarc tzarc tzarc approved these changes

@sigprof sigprof Awaiting requested review from sigprof

Assignees
No one assigned
Labels
awaiting review cli qmk cli command python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@elpekenin @sigprof @tzarc @zvecr