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

uni660rev2 added #8807

Merged
merged 3 commits into from
Apr 24, 2020
Merged

uni660rev2 added #8807

merged 3 commits into from
Apr 24, 2020

Conversation

dededecline
Copy link
Contributor

Description

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

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.
  • 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 a review from a team April 19, 2020 01:22
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

Given keyboards/sirius/uni660/rev2/rev2.h contains both iso and ansi layout macros, the keymap folders keyboards/sirius/uni660/rev2/iso/keymaps and keyboards/sirius/uni660/rev2/ansi/keymaps should be merged to keyboards/sirius/uni660/rev2/keymaps which means updates to readme and such.

@dededecline
Copy link
Contributor Author

@zvecr if I understand how Via reads keymaps correctly, and the fact that the ISO and ANSI PCBs are wired differently rather than just a layout change, we need a separate Via keymap directory for each, correct? That would prevent having a unified keymap directory for rev2.

@zvecr
Copy link
Member

zvecr commented Apr 20, 2020

In which case, keyboards/sirius/uni660/rev2/rev2.h should not contain both LAYOUT macros, and instead a iso.h and ansi.h created in their respective directories. You will also need a config.h for both ansi and iso so that they have different product IDs.

However looking at your LAYOUT macros, you can also go with my original suggestion, one via keymap and use the via layout options to switch between iso and ansi.

@dededecline
Copy link
Contributor Author

The suggested changes have been made

@noroadsleft noroadsleft requested a review from zvecr April 22, 2020 22:12
@zvecr zvecr merged commit 217debf into qmk:master Apr 24, 2020
@zvecr
Copy link
Member

zvecr commented Apr 24, 2020

Thanks!

@dededecline dededecline deleted the uni660rev2 branch April 24, 2020 20:38
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