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

Barleycorn #10118

Merged
merged 26 commits into from
Aug 25, 2020
Merged

Barleycorn #10118

merged 26 commits into from
Aug 25, 2020

Conversation

yiancar
Copy link
Contributor

@yiancar yiancar commented Aug 21, 2020

Description

Adding Ginghams big brother:)

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

@yiancar yiancar mentioned this pull request Aug 21, 2020
6 tasks
@tzarc tzarc requested a review from a team August 22, 2020 03:42
@tzarc tzarc added the keyboard label Aug 22, 2020
keyboards/barleycorn/rules.mk Outdated Show resolved Hide resolved
keyboards/barleycorn/rules.mk Outdated Show resolved Hide resolved
keyboards/barleycorn/rules.mk Outdated Show resolved Hide resolved
@tzarc tzarc requested a review from a team August 22, 2020 03:58
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.

Also, is this one PCB that supports both ANSI and ISO layouts, or are there two PCBs, one ANSI and one ISO?

keyboards/barleycorn/config.h Outdated Show resolved Hide resolved
keyboards/barleycorn/config.h Outdated Show resolved Hide resolved
keyboards/barleycorn/info.json Outdated Show resolved Hide resolved
keyboards/barleycorn/info.json Outdated Show resolved Hide resolved
keyboards/barleycorn/readme.md Outdated Show resolved Hide resolved
keyboards/barleycorn/readme.md Outdated Show resolved Hide resolved
keyboards/barleycorn/rules.mk Outdated Show resolved Hide resolved
keyboards/barleycorn/rules.mk Outdated Show resolved Hide resolved
@noroadsleft noroadsleft requested a review from a team August 22, 2020 17:34
@yiancar
Copy link
Contributor Author

yiancar commented Aug 22, 2020

@noroadsleft Thanks for the review, it supports both ANSI and ISO. Also numpad is variable

Also done with all the requested changes:)

keyboards/barleycorn/readme.md Outdated Show resolved Hide resolved
@yiancar
Copy link
Contributor Author

yiancar commented Aug 22, 2020

@drashna Can you explain QUANTUM_LIB_SRC quickly for future reference?:)

@drashna
Copy link
Member

drashna commented Aug 22, 2020

Absolutely. Or at least, to the best of my ability.

@mtei would be the best one to answer that, since they're the one that added it initially.

For both QUANTUM_LIB_SRC and LIB_SRC, instead of just compiling the file, it adds the file to a library. This prevents issues with it being added multiple times, and can be compiled out, if needed, as well.

Additionally, right now, it also prevents it from being optimized by LTO, which can cause timing and other unexpected issues (such as being optimized out).

So, for stuff like i2c_master, and serial.c, you'd want to use one of these.

Also, it's something that would be best to document this, but I think that mtei has a PR that changes things a bit, and add documentation.

@yiancar
Copy link
Contributor Author

yiancar commented Aug 22, 2020

Cool, really good! yea documentation would be amazing:)

keyboards/barleycorn/matrix.c Outdated Show resolved Hide resolved
keyboards/barleycorn/barleycorn.h Show resolved Hide resolved
@yiancar
Copy link
Contributor Author

yiancar commented Aug 23, 2020

Indeed just get unnecessarily complicated for all the options...

keyboards/barleycorn/barleycorn.c Outdated Show resolved Hide resolved
@yiancar
Copy link
Contributor Author

yiancar commented Aug 24, 2020

@fauxpark #yourwishismycommand ?! anything else?

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.

Looks good to me otherwise.

keyboards/barleycorn/info.json Outdated Show resolved Hide resolved
@yiancar
Copy link
Contributor Author

yiancar commented Aug 24, 2020

I suppose we did great now?:)

@noroadsleft noroadsleft requested a review from a team August 25, 2020 02:07
@noroadsleft noroadsleft merged commit 5460489 into qmk:master Aug 25, 2020
@noroadsleft
Copy link
Member

Thanks!

@yiancar yiancar deleted the houndstooth branch August 25, 2020 09:57
nicocesar pushed a commit to nicocesar/qmk_firmware that referenced this pull request Sep 6, 2020
* Initial commit

* Barleycorn rename

* Update readme.md

* PR checklist fixes

* Update info.json

* Update keyboards/barleycorn/rules.mk

* Update keyboards/barleycorn/config.h

* Update keyboards/barleycorn/rules.mk

* Update keyboards/barleycorn/config.h

* Update keyboards/barleycorn/info.json

* Update keyboards/barleycorn/info.json

* Update keyboards/barleycorn/readme.md

* Update keyboards/barleycorn/rules.mk

* Update keyboards/barleycorn/rules.mk

* PR review

* Update keyboards/barleycorn/readme.md

* Update keyboards/barleycorn/rules.mk

* Update keyboards/barleycorn/keymaps/default/readme.md

* Update keyboards/barleycorn/keymaps/iso/readme.md

* Update keyboards/barleycorn/rules.mk

* Update keyboards/barleycorn/readme.md

* Update keyboards/barleycorn/rules.mk

* Update keyboards/barleycorn/matrix.c

* Update keyboards/barleycorn/barleycorn.c

* added ansi/iso keymaps!

* Update keyboards/barleycorn/info.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants