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

keymap specific build settings #151

Closed
wants to merge 3 commits into from
Closed

keymap specific build settings #151

wants to merge 3 commits into from

Conversation

wez
Copy link
Contributor

@wez wez commented Feb 17, 2016

Three commits in this PR; the overall effect is to allow specifying per-keymap build options.
This is done via an overrides.mk makefile that can options on or off.
I've also added a build-all script to verify that everything builds.
build-all is the kind of thing we can wire up via Travis CI; it takes a few minutes to build out all combinations of the keyboards and keymaps.

Refs: #90

$ ./build-all.sh
<lots of output here>
1 failures:
  make -C keyboard/infinity -f Makefile

I'll submit a separate PR to fix the infinity build (it's an OS X specific problem)

wez added 3 commits February 16, 2016 19:03
This allows setting FOO=no in a makefile and more importantly for me,
allows turning it off by running `make KEYMAP=wez SLEEP_LED_ENABLE=no`
without needing to modify the main Makefile.

We need the strip because our assignments typically look like this:

```
ENABLE_FOO = yes # some comment
```

which results in the variable being assigned as `yes `.  The strip
removes the trailing space.

Refs: qmk#90
You may now create a settings.mk file in a keymap dir.  It is pulled in just
prior to the main quantum.mk and gives you an opportunity to modify the ergodox
configuration.

Refs: qmk#90
The idea is that this helps to detect build breakages by making it
simpler to buid everything.

Some keymaps require specific build options; I've added an overrides.mk
file that can be placed alongside the keyamp file to enable those
options.

Refs: qmk#90

It takes a several minutes to build everything.  I envision that we'd
using Travis CI to run this for each PR and post-commit to surface build
issues and keep the code healthy.
@jackhumbert
Copy link
Member

Thanks for doing this! I'll have to check out the changes in detail later on to make sure it doesn't break anything in the Planck stuff I have setup.

@ezuk
Copy link
Contributor

ezuk commented Mar 27, 2016

@jackhumbert just a quick ping on this one :) Think we could merge/close this week maybe?

@jackhumbert
Copy link
Member

jackhumbert commented Mar 27, 2016

Adding quantum/keymap_extras/ to the path should take care of a lot of the full path includes in a couple of files - I remember doing this in some branch, but I can't remember if I got it merged to master yet.

I like the idea of the two state system (vs two + undef), but I'm gonna miss the ease of #ifdef BLAH - what are your thoughts, @ezuk?

@@ -88,19 +88,19 @@ OPT_DEFS += -DBOOTLOADER_SIZE=512


# Build Options
# comment out to disable the options.
# Set to yes to enable by default, or make MIDI_ENABLE=yes or
# make SLEEP_LED_ENABLE=no to override for a given make invocation
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure i follow this comment; care to rephrase maybe? Do you mean to say commenting out no longer works basically, but I need to specify yes/no? What happens if I do comment it out?

@ezuk
Copy link
Contributor

ezuk commented Mar 27, 2016

@jackhumbert my thoughts are on the noobish side of the spectrum, and I left my sheepish questions as line comments. 🎉 :P

# UNICODE_ENABLE = yes # Unicode

BOOTMAGIC_ENABLE := yes # Virtual DIP switch configuration(+1000)
MOUSEKEY_ENABLE := yes # Mouse keys(+4700)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, and this thing is hard to Google, what's the difference between straight-up = assignment and := assignment?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ezuk I just stumbled across this PR. However, my Google-fu turns up this useful StackOverflow question. It seems to be a question of lazy/immediate evaluation, but I'm not sure how it specifically applies here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that's nice! Thank you!

@jackhumbert
Copy link
Member

@wez thanks for paving the way on this! I was trying to do something similar yesterday, and ended with practically the same code on #256 - it would have been much easier to just merge this :) I think I still have some changes to make to match everything you did.

What do you think about updating this PR to just have the build all script for now?

@ezuk ezuk closed this May 29, 2016
Jpe230 pushed a commit to Jpe230/qmk_firmware that referenced this pull request Dec 15, 2021
Bladehawke pushed a commit to Bladehawke/qmk_firmware that referenced this pull request Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants