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

Cleanup check for PERMISSIVE_HOLD #7861

Merged
merged 2 commits into from
Jan 17, 2020
Merged

Cleanup check for PERMISSIVE_HOLD #7861

merged 2 commits into from
Jan 17, 2020

Conversation

drashna
Copy link
Member

@drashna drashna commented Jan 10, 2020

Description

Extra, unneeded check

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

  • Should have zero impact. Is just cleanup

@drashna drashna requested a review from a team January 10, 2020 20:43
@zvecr zvecr added the bug label Jan 10, 2020
@zvecr
Copy link
Member

zvecr commented Jan 10, 2020

How many people outside the repo have defined PER_KEY_TAPPING_TERM?
If they have, it previously would have skipped the Tapping: End stuff but now it will run it.

Copy link
Contributor

@ridingqwerty ridingqwerty left a comment

Choose a reason for hiding this comment

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

Looks good to me. Chances are very low that anyone defined PER_KEY_TAPPING_TERM since this is the only existence of that phrasing, so it’s unlikely to unexpectedly alter behavior for anyone.

@drashna
Copy link
Member Author

drashna commented Jan 11, 2020

Exactly. It's ... an artifact of me try getting the feature to work, where I changed the name.

It's not used anywhere else.

@zvecr
Copy link
Member

zvecr commented Jan 11, 2020

It's not used anywhere else.

Its not used anywhere else in qmk/qmk_firmware but you cant say 100% its not used by someone outside the repo.

That aside, I do agree that the impact is very low, but i cant see it as zero.

@ridingqwerty ridingqwerty merged commit 1b0854f into master Jan 17, 2020
@drashna drashna deleted the fix/permissive_hold branch January 21, 2020 20:36
drashna added a commit to zsa/qmk_firmware that referenced this pull request Feb 12, 2020
* Fix bug in PERMISSIVE_HOLD check

caused by qmk#5009 (aka, me)

* Remove check for per key
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 14, 2020
* Fix bug in PERMISSIVE_HOLD check

caused by qmk#5009 (aka, me)

* Remove check for per key
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* Fix bug in PERMISSIVE_HOLD check

caused by qmk#5009 (aka, me)

* Remove check for per key
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 26, 2020
* Fix bug in PERMISSIVE_HOLD check

caused by qmk#5009 (aka, me)

* Remove check for per key
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* Fix bug in PERMISSIVE_HOLD check

caused by qmk#5009 (aka, me)

* Remove check for per key
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Fix bug in PERMISSIVE_HOLD check

caused by qmk#5009 (aka, me)

* Remove check for per key
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants