{"action":"created","issue":{"url":"https://api.github.com/repos/jackhumbert/qmk_firmware/issues/452","repository_url":"https://api.github.com/repos/jackhumbert/qmk_firmware","labels_url":"https://api.github.com/repos/jackhumbert/qmk_firmware/issues/452/labels{/name}","comments_url":"https://api.github.com/repos/jackhumbert/qmk_firmware/issues/452/comments","events_url":"https://api.github.com/repos/jackhumbert/qmk_firmware/issues/452/events","html_url":"https://github.com/jackhumbert/qmk_firmware/issues/452","id":162490177,"number":452,"title":"Placing #ifdefs inside or outside of source files","user":{"login":"jackhumbert","id":141431,"avatar_url":"https://avatars.githubusercontent.com/u/141431?v=3","gravatar_id":"","url":"https://api.github.com/users/jackhumbert","html_url":"https://github.com/jackhumbert","followers_url":"https://api.github.com/users/jackhumbert/followers","following_url":"https://api.github.com/users/jackhumbert/following{/other_user}","gists_url":"https://api.github.com/users/jackhumbert/gists{/gist_id}","starred_url":"https://api.github.com/users/jackhumbert/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jackhumbert/subscriptions","organizations_url":"https://api.github.com/users/jackhumbert/orgs","repos_url":"https://api.github.com/users/jackhumbert/repos","events_url":"https://api.github.com/users/jackhumbert/events{/privacy}","received_events_url":"https://api.github.com/users/jackhumbert/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-06-27T16:12:33Z","updated_at":"2016-06-28T02:16:14Z","closed_at":null,"body":"Mentioned in #451, I'd like to look at the possibility of moving a large portion of the\r\n\r\n```c\r\n#ifdef FEATURE_ENABLE\r\n  #include \"feature.h\"\r\n  feature();\r\n#endif\r\n```\r\n\r\ncode blocks to having a wrapper on the entire `feature.c` file to disable it, and include dummy functions where necessary (they likely won't be needed that often).\r\n\r\nPros:\r\n\r\n* disable via `config.h` instead of `Makefile` (doesn't really cleaning, easier/more straight-forward to modify, all being c)\r\n* cleaner code, no giant `#ifdef`s around calls (something `action.c` is suffering from)\r\n* no `#ifdef`s required in `keymap.c`\r\n\r\nCons:\r\n\r\n* is the compiler smart enough to exclude things without affecting memory size?\r\n* code is less clear, since functions will always be called (unsure if they're doing anything)\r\n* `feature.c` will always be compiled (I believe) - does this have an effect on compile time?\r\n\r\nComments are welcome!"},"comment":{"url":"https://api.github.com/repos/jackhumbert/qmk_firmware/issues/comments/228930104","html_url":"https://github.com/jackhumbert/qmk_firmware/issues/452#issuecomment-228930104","issue_url":"https://api.github.com/repos/jackhumbert/qmk_firmware/issues/452","id":228930104,"user":{"login":"jackhumbert","id":141431,"avatar_url":"https://avatars.githubusercontent.com/u/141431?v=3","gravatar_id":"","url":"https://api.github.com/users/jackhumbert","html_url":"https://github.com/jackhumbert","followers_url":"https://api.github.com/users/jackhumbert/followers","following_url":"https://api.github.com/users/jackhumbert/following{/other_user}","gists_url":"https://api.github.com/users/jackhumbert/gists{/gist_id}","starred_url":"https://api.github.com/users/jackhumbert/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jackhumbert/subscriptions","organizations_url":"https://api.github.com/users/jackhumbert/orgs","repos_url":"https://api.github.com/users/jackhumbert/repos","events_url":"https://api.github.com/users/jackhumbert/events{/privacy}","received_events_url":"https://api.github.com/users/jackhumbert/received_events","type":"User","site_admin":false},"created_at":"2016-06-28T02:16:14Z","updated_at":"2016-06-28T02:16:14Z","body":"There's not that many files right now, but when we have 20 similar features that all get compiled no matter what, I'd be a little more concerned with the time it takes to pull each file. I'd really like this format to be the way people can experiment and contribute new behaviors.\r\n\r\nIf we're able to remove the options from the Makefile to the config.h completely, we'll have no need for the `quick` command. The cleaning was added because of the risk of changing the Makefile and the compiler not realising it. \r\n\r\nI'm certain there are ways to clean-up the Makefile a bit - there's some fancier stuff in there that might take a bit longer, especially since some of it gets called quite a few times during compilation. "}}
{"actor":{"display_login":"jackhumbert"}}