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

[Keyboard] Move all helix boards onto split common #16009

Closed
wants to merge 1 commit into from

Conversation

drashna
Copy link
Member

@drashna drashna commented Jan 23, 2022

Description

Core is starting to diverge from custom split implementation. Enough that the pico and rev2 are now erroring out on develop.

Instead of backporting changes, just convert these boards off of custom split implementation and onto split common.

Types of Changes

  • Bugfix
  • Enhancement/optimization
  • Keyboard (addition or update)

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).

@drashna drashna requested review from mtei and a team January 23, 2022 09:39
@drashna
Copy link
Member Author

drashna commented Jan 23, 2022

@MakotoKurauchi Also, if you could test the changes and make sure that nothing is broken?

@fauxpark fauxpark requested a review from a team January 23, 2022 09:42
@mtei
Copy link
Contributor

mtei commented Jan 27, 2022

The following keymap is ready to be migrated to split_common.

  • helix/rev2/default/
  • helix/rev2/five_rows/
  • helix/rev2/led_test/
  • helix/rev2/xulkal/

The keymap below is probably ready to be migrated to split_common. It needs to be tested.

  • helix/rev2/edvorakjp/

The following keymaps depend on local_drivers/ssd1306.c. The OLED-related code in these keymaps needs to be modified to use oled_dirver.c.

  • helix/rev2/five_rows_jis/
  • helix/rev2/fraanrosi/
  • helix/rev2/yshrsmz/

The following keymap is not clear to me; I will discuss it with MakotoKurauchi.

  • helix/rev2/froggy/
  • helix/rev2/froggy_106/

@drashna
Copy link
Member Author

drashna commented Jan 31, 2022

honestly, given the complexity that the helix boards use, that the default keymap compiles ...

@mtei
Copy link
Contributor

mtei commented Feb 16, 2022

I've started work on the Helix switch to split_common, and have started a discussion with MakotoKurauchi.

Please let us create a replacement PR for this one.

@drashna
Copy link
Member Author

drashna commented Feb 16, 2022

I've started work on the Helix switch to split_common, and have started a discussion with MakotoKurauchi.

Please let us create a replacement PR for this one.

Absolutely! I would rather it be that way, to be honest, as I'm nowhere near as familiar with the helix code as you or makotoKurauchi are.

@drashna drashna closed this Feb 16, 2022
@drashna drashna deleted the fix/helix_split_common branch February 16, 2022 18:20
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.

4 participants