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

Remove unused fn_actions[] and action_function() #4829

Merged
merged 1 commit into from Jan 24, 2019
Merged

Remove unused fn_actions[] and action_function() #4829

merged 1 commit into from Jan 24, 2019

Conversation

fauxpark
Copy link
Member

Description

#4820, part 2.

I did a repo-wide search for fn_actions, ignoring anything that isn't a keymap, then went into each result and looked for the regex F[UN\(] (function keycodes appear as F(n), FUNC(n), KC_FNn, or FN_n in the case of layouts that prepend KC_). Any entries in fn_actions that weren't referenced were removed.

Next up I will be switching ACTION_LAYER_MOMENTARY(), ACTION_LAYER_TAP_KEY() and ACTION_LAYER_TAP_TOGGLE() to MO(), LT() and TT() respectively.

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

  • None

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document. (https://docs.qmk.fm/#/contributing)
  • 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).

@drashna drashna added the keymap label Jan 12, 2019
@drashna
Copy link
Member

drashna commented Jan 13, 2019

For now, at least, I think it would be best if these changes were only made to the default layouts.

@fauxpark
Copy link
Member Author

That would be a grand total of 10 keymaps affected.

@fauxpark
Copy link
Member Author

@drashna what should I do with this? Roll it back to just default keymaps?

I'd really like to be rid of this "feature" that duplicates a whole bunch of others :)

@drashna
Copy link
Member

drashna commented Jan 22, 2019

Yeah, for now.

@fauxpark
Copy link
Member Author

I've limited the scope of this PR to just the affected default keymaps, plus the following:

keyboards/cu75/keymaps/iso/keymap.c
keyboards/ergodox_ez/keymaps/testing/keymap.c
keyboards/katana60/keymaps/colemak/keymap.c
keyboards/lfkeyboards/lfk87/keymaps/iso/keymap.c
keyboards/lfkeyboards/smk65/keymaps/iso/keymap.c
layouts/community/ergodox/erez_experimental/keymap.c
layouts/community/ergodox/reset_eeprom/keymap.c
layouts/community/ergodox/robot_test_layout/keymap.c

mechmerlin
mechmerlin approved these changes Jan 24, 2019
View reviewed changes
Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

Thanks!

@mechmerlin mechmerlin merged commit b5feae0 into qmk:master Jan 24, 2019
tenderlove added a commit to tenderlove/qmk_firmware that referenced this pull request Jan 24, 2019
@tenderlove
Merge branch 'master' into merging
e5886c5 
* master: (643 commits)
  [Keyboard] Remove hadron ver0 as it is no longer required (qmk#4921)
  Remove unused fn_actions[] and action_function() in default keymaps (qmk#4829)
  [Keyboard] KBD67: enable bootmagic lite by default (qmk#4931)
  [Keymap] Adding keymap for Nyquist (qmk#4918)
  Optimize/Update the new_project script (qmk#4920)
  Remove lfkeyboards parent rules.mk as its only required for mini1800
  [Keyboard] Add BDN9 (qmk#4919)
  [Keyboard] Add KBD67 Hotswap Support (qmk#4916)
  Fixup the clueboard 66 info.json
  Clueboard refresh (qmk#4902)
  Give the keymap folder the proper name
  Fix layouts/default/66_iso keymap
  [Keyboard] Add DataHand keyboard support (qmk#4847)
  [Keymap] Add a compile-time provided macro and assign to _FL (qmk#4908)
  Added info.json for TGR Alice
  Always read two bytes from the endpoint if we have two bytes to read
  Rename i2c_slave functions so it can coexist with i2c_master (qmk#4875)
  Fix for ISO layout in tada68:rys (qmk#4906)
  [Keyboard] Added TGR Alice keyboard support (qmk#4896)
  handwired/retro_refit: refactor, Configurator support and readme update (qmk#4899)
  ...
@fauxpark fauxpark deleted the remove-unused-fn-actions branch January 24, 2019 21:33
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Jan 25, 2019
@loadedsith
Merge branch 'master' into grahampheath
e372e24 
* master: (60 commits)
  [Keymap] New kbd67/hotswap keymap for writing both code and math (qmk#4933)
  Add support for Clueboard 66% rev4
  Fix a typo in link to the Pro Micro ISP firmware
  [Keyboard] Remove hadron ver0 as it is no longer required (qmk#4921)
  Remove unused fn_actions[] and action_function() in default keymaps (qmk#4829)
  [Keyboard] KBD67: enable bootmagic lite by default (qmk#4931)
  [Keymap] Adding keymap for Nyquist (qmk#4918)
  Optimize/Update the new_project script (qmk#4920)
  Remove lfkeyboards parent rules.mk as its only required for mini1800
  [Keyboard] Add BDN9 (qmk#4919)
  [Keyboard] Add KBD67 Hotswap Support (qmk#4916)
  Fixup the clueboard 66 info.json
  Clueboard refresh (qmk#4902)
  Give the keymap folder the proper name
  Fix layouts/default/66_iso keymap
  [Keyboard] Add DataHand keyboard support (qmk#4847)
  [Keymap] Add a compile-time provided macro and assign to _FL (qmk#4908)
  Added info.json for TGR Alice
  Always read two bytes from the endpoint if we have two bytes to read
  Rename i2c_slave functions so it can coexist with i2c_master (qmk#4875)
  ...
zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Feb 2, 2019
@fauxpark @zer09
Remove unused fn_actions[] and action_function() in default keymaps (q…
6dd5799 
…mk#4829)
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
@fauxpark
Remove unused fn_actions[] and action_function() in default keymaps (q…
d25b713 
…mk#4829)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drashna drashna drashna approved these changes

@mechmerlin mechmerlin mechmerlin approved these changes

@ezuk ezuk Awaiting requested review from ezuk

@fredizzimo fredizzimo Awaiting requested review from fredizzimo

@jackhumbert jackhumbert Awaiting requested review from jackhumbert

@skullydazed skullydazed Awaiting requested review from skullydazed

Assignees
No one assigned
Labels
keymap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fauxpark @drashna @mechmerlin