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

Move keymap-specific Makefile options to keymaps somehow #90

Closed
jackhumbert opened this issue Jan 15, 2016 · 9 comments
Closed

Move keymap-specific Makefile options to keymaps somehow #90

jackhumbert opened this issue Jan 15, 2016 · 9 comments

Comments

@jackhumbert
Copy link
Member

No description provided.

@jackhumbert
Copy link
Member Author

Action in the Makefile that scans the keymap being compiled for a USING_UNICODE, and overwrites the option in the Makefile?

@ezuk
Copy link
Contributor

ezuk commented Jan 16, 2016

I don't love scanning like this because it might get triggered by mistake or something. Thinking about how we might integrate this with Fusion, how about we go with a -f as described in the Make docs?

So for example, if I wanted to specify a custom make file for keymap_oscar, I'd call it make_keymap_oscar, stick it right alongside the firmware, and in the README for that keymap (which I try to get everyone to include before I merge their keymaps in), I'd specify how to invoke make with it.

What do you think?

@jackhumbert
Copy link
Member Author

We could do that, and have the custom Makefile include the common one - we'd need to stick some if statements in the common Makefile to accommodate that.

But I'm imagining a first line comment similar to:

 # REQUIRES_UNICODE

That would make it pretty obvious to the user, and we could output some comments saying the current options have been overridden.

@jackhumbert
Copy link
Member Author

This makes things a little more organised as well, keeping things all in one file. Honestly, I'd be interested in merging the readmes into the .c files.

@ezuk
Copy link
Contributor

ezuk commented Jan 16, 2016

The reason I like the having the Readme.md alongside the keymap is how github renders it. I have a lot of people who aren't super comfortable with C look into the repo, and I want to make it easy for them to understand what each keymap does, and choose the one they want (that's also why I like it when people check in hex files).

So my ideal keymap goes something like:

  • keymap_moo.c
  • keymap_moo.png (image showing it, not a must of course)
  • keymap_moo_README.md
  • keymap_moo.hex

That's just for my own keyboard obviously, but I find that it helps people understand what they're getting without becoming confused. I'd rather have multiple files, each doing something specific, than one big file that tries to be both code and docs.

As for the makefile issue... Well, could go either way. I'm not so sure about the directives but if that's the direction you find more compelling, sure, let's do that. :)

@jackhumbert
Copy link
Member Author

Got a working version up in the smarkefile branch :)

Right now, you can throw USING_UNICODE, USING_MIDI, or USING_BACKLIGHT anywhere in your keymap file, and it'll include the correct stuff/set the flags. I put it as a comment like this, but I'm open to ideas:

// USING_MIDI

What do you think about organising those keymaps in folders? That would allow github to display the README more easily. The structure could be:

  • keymap_moo/ (or even moo/)
    • README.md
    • default.c
    • default.hex
    • default.png

That'd also allow folks to have alternative versions more easily. I haven't tested it, but we should be able to do make KEYMAP=keymap_moo/default without breaking anything.

@jackhumbert
Copy link
Member Author

Testing this out here - I have a smart Makefile looking to see if <name>.c exists, and if it doesn't, it pulls <name>/default.c. If neither exists, it errors out.

@ezuk
Copy link
Contributor

ezuk commented Jan 16, 2016

I love the idea of using a folder structure! Will try something out over the next few days. Left a comment on that commit re the file scanning (breaks cross-platform).

@jackhumbert
Copy link
Member Author

Dang.. I guess that's where late-night coding gets me. There might be some better cross-platform tools available - I'll circle back around with this!

wez added a commit to wez/qmk_firmware that referenced this issue Feb 17, 2016
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
wez added a commit to wez/qmk_firmware that referenced this issue Feb 17, 2016
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
wez added a commit to wez/qmk_firmware that referenced this issue Feb 17, 2016
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.
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

No branches or pull requests

2 participants