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

add missing physical layout options and VIA support for Sesame #10471

Merged
merged 5 commits into from
Nov 8, 2020
Merged

add missing physical layout options and VIA support for Sesame #10471

merged 5 commits into from
Nov 8, 2020

Conversation

str-dst
Copy link
Contributor

@str-dst str-dst commented Sep 28, 2020

Description

Added the missing physical layout options and added VIA support for the Sesame Alice keyboard

Types of Changes

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

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

fauxpark
fauxpark requested changes Oct 1, 2020
View reviewed changes
keyboards/kb_elmo/sesame/info.json Outdated Show resolved Hide resolved
keyboards/kb_elmo/sesame/info.json Outdated Show resolved Hide resolved
keyboards/kb_elmo/sesame/info.json Outdated Show resolved Hide resolved
keyboards/kb_elmo/sesame/info.json Outdated Show resolved Hide resolved
keyboards/kb_elmo/sesame/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/kb_elmo/sesame/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/kb_elmo/sesame/keymaps/via/keymap.c Outdated Show resolved Hide resolved
@fauxpark fauxpark added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Oct 1, 2020
@fauxpark fauxpark requested a review from a team October 1, 2020 18:17
noroadsleft
noroadsleft requested changes Oct 1, 2020
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

I'm not keen on the fact that the macro that was LAYOUT layout is now LAYOUT_split_rshift, and a new LAYOUT is in its place. This breaks keymaps for any users who have their source out of tree (a local qmk_firmware fork without their keymap tracked by Git) or uses QMK Configurator.

If the new LAYOUT were to be renamed, and a #define LAYOUT LAYOUT_split_rshift added, I'd be okay with that.

@str-dst
Copy link
Contributor Author

str-dst commented Oct 1, 2020

I'm not keen on the fact that the macro that was LAYOUT layout is now LAYOUT_split_rshift, and a new LAYOUT is in its place. This breaks keymaps for any users who have their source out of tree (a local qmk_firmware fork without their keymap tracked by Git) or uses QMK Configurator.

If the new LAYOUT were to be renamed, and a #define LAYOUT LAYOUT_split_rshift added, I'd be okay with that.

This is an open source project keyboard that I designed and it was never publicly sold since it is still under development.
There are maybe a dozen people that have prototypes of this board and I doubt that anyone of those people uses QMK user layouts.
The standard QMK user layouts won't work with this anyways since the keyboard is an Alice style layout.

So I don't think that this is in any way relevant for this keyboard.

@str-dst @fauxpark
Apply suggestions from code review
9b207b4 
Co-authored-by: Ryan <fauxpark@gmail.com>
@str-dst
Copy link
Contributor Author

str-dst commented Oct 18, 2020

After talking with a few of the "early adopters" of the board I decided to make the split right shift the standard layout again.
The full shift is now on a new layout named LAYOUT_long_rshift.

@drashna drashna requested a review from a team October 18, 2020 20:40
@str-dst str-dst mentioned this pull request Oct 21, 2020
Merged
6 tasks
@fauxpark fauxpark requested a review from a team October 27, 2020 08:31
@noroadsleft noroadsleft merged commit 9cd3ffa into qmk:master Nov 8, 2020
@noroadsleft
Copy link
Member

Thanks!

ChrissiQ pushed a commit to ChrissiQ/qmk_firmware that referenced this pull request Nov 10, 2020
@str-dst @ChrissiQ
add missing physical layout options and VIA support for Sesame (qmk#1…
3003ba8 
…0471)

* add missing physical layout options and VIA support for Sesame

* Apply suggestions from code review

* make split rshift standard again
nicsuzor added a commit to nicsuzor/qmk_firmware that referenced this pull request Nov 14, 2020
@nicsuzor
Merge remote-tracking branch 'upstream/master'
9133835 
* upstream/master: (48 commits)
  [Keymap] idobo:egstad (qmk#10783)
  Adding few Korean translated files (qmk#5895)
  [Keyboard] Keebwerk MEGA Initial commit (qmk#10777)
  Indicator LEDs as config (qmk#10816)
  add missing physical layout options and VIA support for Sesame (qmk#10471)
  [Keyboard] Fix unused variables in mschwingen modelm (qmk#10811)
  Add big spacebar defaults to Underscore33 (qmk#10731)
  Add qmk info -l to show the layouts too (qmk#10882)
  New command: qmk lint (qmk#10761)
  Updates to Talljoe's Keymaps (qmk#10115)
  [Keymap] bcat keymaps and userspace (qmk#10705)
  add dp60 indicator mode (qmk#8801)
  E85 backlight & LED indicator updates (qmk#10678)
  Add support for 4 IS31FL3731 devices (qmk#10860)
  [Keymap] add brandonschlack userspace and keymaps (qmk#10411)
  [Keymap] add ai03/polaris:mekberg (qmk#10508)
  CLI: Add `qmk clean` (qmk#10785)
  Adds support for XD84 Pro (qmk#9750)
  Freyr refactor (qmk#10833)
  KC60 refactor (qmk#10834)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noroadsleft noroadsleft noroadsleft approved these changes

@fauxpark fauxpark fauxpark approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@str-dst @noroadsleft @drashna @fauxpark