-
-
Notifications
You must be signed in to change notification settings - Fork 39.6k
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
gcc 10 compatibility for Drop alt #9485
Conversation
According to the official documentation[1] gcc 10 is more strict about correct extern usage. I've had to move the definition of dmac_desc and dmac_desc_wb from i2c_master.h to the corresponding .c file. This could be an issue if anyone includes the i2c_master.h file without liking with the object file. [1]: https://gcc.gnu.org/gcc-10/porting_to.html
The keymap_config def was conflicting with the one found in tmk_core/common/magic.c. Declaring it extern in magic.c breaks a bunch of keyboard that rely on that declaration (like the ergodox). Instead I've removed the one found in the keymap.c of the massdrop alt. The same change will have to be made to other keyboards.
For good measure I should probably say that compiling with |
@zvecr Could i ask what the breaking change here is? Keymaps that don't remove That is to say, people that don't want to remove |
From the contributing guide which you have ticked as read,
And also https://docs.qmk.fm/#/breaking_changes_instructions,
|
Fair enough. I figured the since the change didn't really change anything that mattered it was ok, but I see the merge angle being a problem for non-dev users. Would it be better if I split the changes into an i2c part and a keymap part? that way the i2c part wont be breaking. Another option would be to forego changing any keymaps except default and let the users change them themselves. Do you have a preference for either, or should I just leave it as is? |
That may be, but issues arise when users want to update their repo. Not everyone is savvy with git, and handling merge conflicts. We want to reduce that, or at least reduce the frequency of it happening. And I think the best option would be to target |
Thanks to this PR I could compile my Drop CTRL |
If we get signoff from @abishalom, @bontakun, @favorable-mutation and @reywood, this can go straight into master. |
LGTM! Thanks for adding this fix! |
👍 looks good |
@abishalom @bontakun any objections? |
Fine by me |
Looks like it'll need to go to |
* Split dmac_desc declaration and definition According to the official documentation[1] gcc 10 is more strict about correct extern usage. I've had to move the definition of dmac_desc and dmac_desc_wb from i2c_master.h to the corresponding .c file. This could be an issue if anyone includes the i2c_master.h file without liking with the object file. [1]: https://gcc.gnu.org/gcc-10/porting_to.html * Remove the keymap_config definition from keymaps The keymap_config def was conflicting with the one found in tmk_core/common/magic.c. Declaring it extern in magic.c breaks a bunch of keyboard that rely on that declaration (like the ergodox). Instead I've removed the one found in the keymap.c of the massdrop alt. The same change will have to be made to other keyboards.
According to the official documentation[1] gcc 10 is more strict about
correct extern usage.
I've had to split the definition and declaration of dmac_desc and dmac_desc_wb
from i2c_master.{c,h}. This could be an issue if anyone includes the
i2c_master.h file without liking with the object file.
I've also had to remove the keymap_config definition from all the massdrop alt
keymaps, since it's defined in magic.c as well. The same would have to be done
for other keymaps. It doesn't seem like any of the in-tree keymaps use the
variable anyway. Anyone using it would have to declare it extern in their
keymap.
With these changes all the in-tree keymaps for the massdrop alt now compiles in
gcc 10
Types of Changes
Issues Fixed or Closed by This PR
Checklist