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

Move Iris via support to Via keymap #4893

Merged
merged 3 commits into from
Jan 27, 2019
Merged

Move Iris via support to Via keymap #4893

merged 3 commits into from
Jan 27, 2019

Conversation

drashna
Copy link
Member

@drashna drashna commented Jan 20, 2019

Description

And move it out of the main keyboard files

Otherwise, it's breaking the keyboard for a bunch of users, myself included.

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.
  • 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
Move Iris via support to Via keymap
ab1db1f 
And move it out of the main keyboard files
@drashna
Copy link
Member Author

drashna commented Jan 20, 2019

@nooges

@drashna
Copy link
Member Author

drashna commented Jan 20, 2019

Travis CI errors are unrelated to this change.
https://travis-ci.org/qmk/qmk_firmware/builds/482149913#L919-L928

Specifically, old compiler version causing inflated file sizes.

@wilba
Copy link
Contributor

wilba commented Jan 21, 2019

I don't see the via keymap. Could be cos it's Monday morning here, though ;-)

@nooges
Copy link
Member

nooges commented Jan 21, 2019

Yeah I don't see it either

@drashna
Copy link
Member Author

drashna commented Jan 21, 2019 •
edited
Loading

Nope, that's my bad, and me being super tired today. Give me a bit.

Edit: There, that should have the keymaps. Sorry about that!

@drashna
Actually add via keymap folder
e446abb 
Because using git when you're tired is apparently a very bad idea, and you do stupid things like not running git add
@wilba
Copy link
Contributor

wilba commented Jan 21, 2019

I think the config.h in the via keymap is redundant, the one in iris should have all that is needed for dynamic keymaps if enabled.

@drashna drashna mentioned this pull request Jan 23, 2019
Closed
13 tasks
@drashna
Copy link
Member Author

drashna commented Jan 23, 2019

Well, the config.h file is a direct copy from the default keymap.

The USE_SERIAL, and MASTER_LEFT should be left in, since they control how communication is handled, and the orientation of the actual keyboard. Though, without these, it defaults to these settings. But leaving them in makes it easier to reference if somebody uses the keymap and changes it.

As for the RGB stuff, most of that could be removed, as it will default to the built in values. The one that shouldn't is the RGBLIGHT_ANIMATION, since that's not defined in the keyboard's config.h file, so removing it would actually disable animations for the keyboard.

@nooges
Copy link
Member

nooges commented Jan 23, 2019

Acutally, USE_SERIAL and MASTER_LEFT can be taken out (not just here, but everywhere), as they're not used at all in split_common. Everything else is good to go.

@drashna
Copy link
Member Author

drashna commented Jan 23, 2019 •
edited
Loading

Okay

nooges
nooges reviewed Jan 23, 2019
View reviewed changes
Copy link
Member

@nooges nooges left a comment

Choose a reason for hiding this comment

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

👍 🥇

@nooges
Copy link
Member

nooges commented Jan 27, 2019

Can we get this merged in?

@drashna drashna merged commit 5d26ebc into qmk:master Jan 27, 2019
@drashna drashna deleted the fix/iris branch January 27, 2019 08:09
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Jan 27, 2019
@Shinichi-Ohki
Merge branch 'master' of https://github.com/qmk/qmk_firmware
b7f634d 
* 'master' of https://github.com/qmk/qmk_firmware:
  Fix Command feature: use get_mods() instead of keyboard_report->mods (qmk#4955)
  [Keymap] Small improvements to Maxr1998's Contra keymap (qmk#4952)
  [Keymap] Minor updates to my dz60 keymap (qmk#4943)
  [Keyboard] UniGo66 keyboard added (qmk#4913)
  [Keymap] Move Iris via support to Via keymap (qmk#4893)
  Adds a default value for IS_COMMAND for COMMAND feature (qmk#4301)
  [Keyboard] drop unused i2c files (qmk#4948)
  [Keymap] Add Maxim keymap for Fourier (qmk#4534)
  use built-in arm stuff
  [Keymap] Add userspace files for vosechu (qmk#4912)
zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Feb 9, 2019
@drashna @zer09
[Keymap] Move Iris via support to Via keymap (qmk#4893)
78a2652 
* Move Iris via support to Via keymap

And move it out of the main keyboard files

* Actually add via keymap folder

Because using git when you're tired is apparently a very bad idea, and you do stupid things like not running git add

* Clean up config.h file for iris via keymap
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
@drashna
[Keymap] Move Iris via support to Via keymap (qmk#4893)
7db768f 
* Move Iris via support to Via keymap

And move it out of the main keyboard files

* Actually add via keymap folder

Because using git when you're tired is apparently a very bad idea, and you do stupid things like not running git add

* Clean up config.h file for iris via keymap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nooges nooges nooges left review comments

@jackhumbert jackhumbert Awaiting requested review from jackhumbert

@ezuk ezuk Awaiting requested review from ezuk

@skullydazed skullydazed Awaiting requested review from skullydazed

@fredizzimo fredizzimo Awaiting requested review from fredizzimo

@mechmerlin mechmerlin Awaiting requested review from mechmerlin

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

Successfully merging this pull request may close these issues.
3 participants
@drashna @wilba @nooges