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

QWERTYYdox refactor #6446

Merged
merged 13 commits into from
Aug 1, 2019
Merged

QWERTYYdox refactor #6446

merged 13 commits into from
Aug 1, 2019

Conversation

noroadsleft
Copy link
Member

Description

Noticed some weirdness while making a QMK Configurator default keymap. Investigated and found some issues with the existing codebase.

Changes were checked by compiling the default keymap from branch base and then again from the development branch and comparing the SHA-256 hashes (which are the same).

External Reference

Types of Changes

  • Enhancement/optimization
  • Keyboard (addition or update)

Checklist

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

`__` key in keymap.c doesn't actually exist on the physical hardware.
Removed key from keymap.c and removed its argument from the layout macro.
It makes the keymap pretty.
- convert to pragma once include guard
- remove redundant definitions
- remove commented code blocks
Was copied from ergotravel; not valid for this keyboard.
Previous codebase enabled Backlight at keyboard level then disabled it at revision level.
Aligns the keyboard-level config.h file more closely with the current QMK template for AVR keyboards.
@aydenvis
Copy link
Contributor

Does this mean that the QWERTTYdox files are fully up to date with the latest split firmware?

Copy link
Contributor

@aydenvis aydenvis left a comment

Choose a reason for hiding this comment

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

You've gone through and cleaned up a lot, doing what I was too afraid to do. Thanks man!

@noroadsleft
Copy link
Member Author

Does this mean that the QWERTTYdox files are fully up to date with the latest split firmware?

Yes. You were already using the Split Common architecture (enabled via SPLIT_KEYBOARD = yes in rules.mk), so consider that a gift from Past You to Present You. 😄

Pretty much all of these changes are coding convention and implementation details. Realistically, as you only have one revision, you don't actually need the rev1 folder, but I saw on your pull request that added this keyboard (#3636) that by your own admission you weren't great at GitHub, so I tried to break the commits down into small, related changes, so you could read each commit in order and follow the logic (which I consider a good practice anyway), and I also didn't delete the rev1 folder so the make command for compiling the firmware is still the same.

Ideally for this keyboard, all of the code should have been in keyboards/qwertyydox/ instead of being split between that folder and keyboards/qwertyydox/rev1/, but as the ErgoTravel was "the source" of your code and did this too, I don't blame you for this; adding a new keyboard can be intimidating. Revision folders can always be added later if you change the PCB in a way where it can't run the same firmware.

You've gone through and cleaned up a lot, doing what I was too afraid to do. Thanks man!

You're welcome, friend.

Copy link
Member

@drashna drashna 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!

@drashna drashna merged commit 9609ae6 into qmk:master Aug 1, 2019
@noroadsleft noroadsleft deleted the cf/qwertyydox branch August 1, 2019 17:30
raymond-w-ko pushed a commit to raymond-w-ko/qmk_firmware that referenced this pull request Aug 4, 2019
* Delete null key

`__` key in keymap.c doesn't actually exist on the physical hardware.
Removed key from keymap.c and removed its argument from the layout macro.

* Delete unused keycode aliases

* Replace layer index definitions with an enum

* Replace redundant numpad keycodes with native aliases

* Use native layer change keycodes instead of aliases

* Visually align the keycodes

It makes the keymap pretty.

* Correct Configurator layout data

* Clean up header files

- convert to pragma once include guard
- remove redundant definitions
- remove commented code blocks

* Delete LAYOUT_kc macro

Was copied from ergotravel; not valid for this keyboard.

* Consolidate rev1 rules.mk settings to keyboard level

Previous codebase enabled Backlight at keyboard level then disabled it at revision level.

* Delete unused rules

* Consolidate config.h settings from keymap level to keyboard level

* Modernize keyboard's config.h file

Aligns the keyboard-level config.h file more closely with the current QMK template for AVR keyboards.
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Aug 6, 2019
* 'master' of https://github.com/qmk/qmk_firmware: (240 commits)
  Move maartenwut's keyboards to one folder (qmk#6484)
  GH60 Refactor: Move Satan into GH60 directory (qmk#6485)
  Remove unused _BOOTLOADER defines
  [Keyboard] add keyboard-discipline (qmk#6464)
  [Split] Add config option for DIRECT_PINS_RIGHT (qmk#6479)
  [Keymap] Add some more commonly used symbols to melody96/zunger. (qmk#6478)
  [Keyboard] WT75-A & WT75-B fixes for QMK Configurator (qmk#6472)
  [Keymap] update to dsanchezseco keymap (qmk#6470)
  [Keymap] Fixed Tanuki RGB lighting (qmk#6462)
  Add iS0 Keypad (qmk#6456)
  [Keymap] jotix ortho_4x12_layout (qmk#6458)
  [Keyboard] New Keyboard: KBDPad MKI (qmk#6452)
  [Keymap] Adds Keymap for Iris/blucky (qmk#6449)
  [Keymap] Fixing Sol Rev2 default keymap OLED rotation. (qmk#6448)
  [Keyboard] QWERTYYdox refactor (qmk#6446)
  [Keymap] Adds keymap for muzfuz/lunar (qmk#6432)
  [Docs] Add dfu-util and caterina udev examples (qmk#6429)
  updates to akb/raine in ready for production units (qmk#6457)
  Removed print call to resolve  qmk#6364 (qmk#6413)
  Fix typo for building Ergodox EZ keyboards (qmk#6453)
  ...
doughsay pushed a commit to doughsay/qmk_firmware that referenced this pull request Aug 31, 2019
* Delete null key

`__` key in keymap.c doesn't actually exist on the physical hardware.
Removed key from keymap.c and removed its argument from the layout macro.

* Delete unused keycode aliases

* Replace layer index definitions with an enum

* Replace redundant numpad keycodes with native aliases

* Use native layer change keycodes instead of aliases

* Visually align the keycodes

It makes the keymap pretty.

* Correct Configurator layout data

* Clean up header files

- convert to pragma once include guard
- remove redundant definitions
- remove commented code blocks

* Delete LAYOUT_kc macro

Was copied from ergotravel; not valid for this keyboard.

* Consolidate rev1 rules.mk settings to keyboard level

Previous codebase enabled Backlight at keyboard level then disabled it at revision level.

* Delete unused rules

* Consolidate config.h settings from keymap level to keyboard level

* Modernize keyboard's config.h file

Aligns the keyboard-level config.h file more closely with the current QMK template for AVR keyboards.
swanmatch pushed a commit to swanmatch/qmk_firmware that referenced this pull request Sep 3, 2019
* Delete null key

`__` key in keymap.c doesn't actually exist on the physical hardware.
Removed key from keymap.c and removed its argument from the layout macro.

* Delete unused keycode aliases

* Replace layer index definitions with an enum

* Replace redundant numpad keycodes with native aliases

* Use native layer change keycodes instead of aliases

* Visually align the keycodes

It makes the keymap pretty.

* Correct Configurator layout data

* Clean up header files

- convert to pragma once include guard
- remove redundant definitions
- remove commented code blocks

* Delete LAYOUT_kc macro

Was copied from ergotravel; not valid for this keyboard.

* Consolidate rev1 rules.mk settings to keyboard level

Previous codebase enabled Backlight at keyboard level then disabled it at revision level.

* Delete unused rules

* Consolidate config.h settings from keymap level to keyboard level

* Modernize keyboard's config.h file

Aligns the keyboard-level config.h file more closely with the current QMK template for AVR keyboards.
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
* Delete null key

`__` key in keymap.c doesn't actually exist on the physical hardware.
Removed key from keymap.c and removed its argument from the layout macro.

* Delete unused keycode aliases

* Replace layer index definitions with an enum

* Replace redundant numpad keycodes with native aliases

* Use native layer change keycodes instead of aliases

* Visually align the keycodes

It makes the keymap pretty.

* Correct Configurator layout data

* Clean up header files

- convert to pragma once include guard
- remove redundant definitions
- remove commented code blocks

* Delete LAYOUT_kc macro

Was copied from ergotravel; not valid for this keyboard.

* Consolidate rev1 rules.mk settings to keyboard level

Previous codebase enabled Backlight at keyboard level then disabled it at revision level.

* Delete unused rules

* Consolidate config.h settings from keymap level to keyboard level

* Modernize keyboard's config.h file

Aligns the keyboard-level config.h file more closely with the current QMK template for AVR keyboards.
ridingqwerty pushed a commit to ridingqwerty/qmk_firmware that referenced this pull request Jan 10, 2020
* Delete null key

`__` key in keymap.c doesn't actually exist on the physical hardware.
Removed key from keymap.c and removed its argument from the layout macro.

* Delete unused keycode aliases

* Replace layer index definitions with an enum

* Replace redundant numpad keycodes with native aliases

* Use native layer change keycodes instead of aliases

* Visually align the keycodes

It makes the keymap pretty.

* Correct Configurator layout data

* Clean up header files

- convert to pragma once include guard
- remove redundant definitions
- remove commented code blocks

* Delete LAYOUT_kc macro

Was copied from ergotravel; not valid for this keyboard.

* Consolidate rev1 rules.mk settings to keyboard level

Previous codebase enabled Backlight at keyboard level then disabled it at revision level.

* Delete unused rules

* Consolidate config.h settings from keymap level to keyboard level

* Modernize keyboard's config.h file

Aligns the keyboard-level config.h file more closely with the current QMK template for AVR keyboards.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants