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

Remove build option firmware size impacts #6947

Merged
merged 7 commits into from
Oct 19, 2019

Conversation

amberstarlight
Copy link
Contributor

Description

Removes references to build option sizes in each keyboard's rules.mk (e.g. (+1000)).

Includes the script used to do this in util/, rules_cleanup.sh.

I have also removed references to these sizes in the documentation, where appropriate.

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

Fixes #5488 - as @fauxpark comments;

These numbers are misleading, because they are outdated and as @drashna mentioned here: #5485 (comment), dependent on the compiler version. It seems there is not much point having them around since they will simply never be accurate.

Checklist

  • My code follows the code style of this project.
  • 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).

keyboards/40percentclub/ut47/rules.mk Outdated Show resolved Hide resolved
@fauxpark
Copy link
Member

fauxpark commented Oct 7, 2019

Nice work!

It's going to be way easier to review this if you split it up into manageable chunks, eg. alphabetically. Additionally, the modification of non-default keymaps will make it a breaking change, so you may want to either exclude the keymaps/ directories from the script, or manually filter out anything that isn't default-ish.

@amberstarlight
Copy link
Contributor Author

@fauxpark thanks!

I've fixed the regex in the script (thanks for spotting @zvecr) and excluded the keymaps/ directories from it. Happy to split this into multiple PRs, as you did to remove boilerplate (e.g. #6804).

My thinking is to create 4 branches at this stage for keyboard directories;

  • A-F (including 0-9, which is only 9 keyboard directories)
  • G-L
  • M-R
  • S-Z

Does this sound good?

@noroadsleft
Copy link
Member

More branches and smaller chunks might be easier to review. G through L I expect would be particularly large due to all the handwired boards and the KBDfans and Keebio vendor folders.

@fauxpark
Copy link
Member

fauxpark commented Oct 7, 2019

Yeah I think 0-9, then each letter individually, though some letters will have only a handful of boards so you can potentially combine them in places. This is what I'm going to do with the boilerplate stuff.

quantum/template/avr/rules.mk Outdated Show resolved Hide resolved
@@ -0,0 +1,40 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Actually, we've been trying to move to a python based CLI system, and ideally, this should be added to the qmk cli code, if possible.

https://docs.qmk.fm/#/cli_development

If you don't think you can do that, that's fine, no worries. But if you're able to, awesome!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not au fait with Python, so I'd have to take a look at this in the future after some learning.

@drashna drashna requested review from zvecr, fauxpark and a team October 16, 2019 17:59
Fixed missing tabs/spaces.

Co-Authored-By: fauxpark <[email protected]>
@fauxpark fauxpark mentioned this pull request Oct 18, 2019
13 tasks
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Thanks!

@drashna drashna merged commit b23f601 into qmk:master Oct 19, 2019
@amberstarlight amberstarlight deleted the firmware-size-impacts-removal branch October 21, 2019 15:41
@fauxpark fauxpark mentioned this pull request Nov 11, 2019
13 tasks
monksoffunk added a commit to monksoffunk/qmk_firmware that referenced this pull request Nov 11, 2019
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
* Update rules.mk template to remove build option size impacts

* Add rules.mk cleaning script

* Update all rules.mk files to remove build option firmware size impact messages

* Remove references to feature filesize in documentation

* Revert "Update all rules.mk files to remove build option firmware size impact messages"

This reverts commit 7cfe709.

* Fix regex in cleanup script and exclude keymaps/ directories

* Update quantum/template/avr/rules.mk

Fixed missing tabs/spaces.

Co-Authored-By: fauxpark <[email protected]>
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* Update rules.mk template to remove build option size impacts

* Add rules.mk cleaning script

* Update all rules.mk files to remove build option firmware size impact messages

* Remove references to feature filesize in documentation

* Revert "Update all rules.mk files to remove build option firmware size impact messages"

This reverts commit 7cfe709.

* Fix regex in cleanup script and exclude keymaps/ directories

* Update quantum/template/avr/rules.mk

Fixed missing tabs/spaces.

Co-Authored-By: fauxpark <[email protected]>
noroadsleft pushed a commit that referenced this pull request May 4, 2020
* Add Zinc keyboard

* Fix RGB LED init of monks/keymap.c

* Add RGBLED_BOTH_ENABLE option

* Fix RGBLED_BOTH_ENABLE option

* Add LED_BOTH_ENABLE feature to 2 keymaps

* Use split_common instead of own split flies

* Fix split LED

* Fix RGB LED config for iOS device

* Add RGB_MOD reverse key to default keymap

* Update readme_jp.md of default keymap

* Add readme_en.md of default keymap

* Merge branch 'master' into zinc_splitcommon

# Conflicts:
#	keyboards/zinc/serial.c

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/rules.mk

* Update keyboards/zinc/rules.mk

* Update toshi0383 keymap

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Remove build option firmware size impacts (as #6947)

* Remove some dead code and whitespace

* Remove unused code

* Remove unused code

* Update keyboards/zinc/rev1/config.h

* Update keyboards/zinc/reva/config.h

* Update keyboards/zinc/keymaps/default/rules.mk

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/readme_en.md

* Update keyboards/zinc/keymaps/default/readme_en.md

* Breaking Changes Changelog
noroadsleft pushed a commit that referenced this pull request May 15, 2020
* Add Zinc keyboard

* Fix RGB LED init of monks/keymap.c

* Add RGBLED_BOTH_ENABLE option

* Fix RGBLED_BOTH_ENABLE option

* Add LED_BOTH_ENABLE feature to 2 keymaps

* Use split_common instead of own split flies

* Fix split LED

* Fix RGB LED config for iOS device

* Add RGB_MOD reverse key to default keymap

* Update readme_jp.md of default keymap

* Add readme_en.md of default keymap

* Merge branch 'master' into zinc_splitcommon

# Conflicts:
#	keyboards/zinc/serial.c

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/rules.mk

* Update keyboards/zinc/rules.mk

* Update toshi0383 keymap

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Remove build option firmware size impacts (as #6947)

* Remove some dead code and whitespace

* Remove unused code

* Remove unused code

* Update keyboards/zinc/rev1/config.h

* Update keyboards/zinc/reva/config.h

* Update keyboards/zinc/keymaps/default/rules.mk

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/readme_en.md

* Update keyboards/zinc/keymaps/default/readme_en.md

* Breaking Changes Changelog
noroadsleft pushed a commit to noroadsleft/qmk_firmware that referenced this pull request May 22, 2020
* Add Zinc keyboard

* Fix RGB LED init of monks/keymap.c

* Add RGBLED_BOTH_ENABLE option

* Fix RGBLED_BOTH_ENABLE option

* Add LED_BOTH_ENABLE feature to 2 keymaps

* Use split_common instead of own split flies

* Fix split LED

* Fix RGB LED config for iOS device

* Add RGB_MOD reverse key to default keymap

* Update readme_jp.md of default keymap

* Add readme_en.md of default keymap

* Merge branch 'master' into zinc_splitcommon

# Conflicts:
#	keyboards/zinc/serial.c

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/rules.mk

* Update keyboards/zinc/rules.mk

* Update toshi0383 keymap

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Remove build option firmware size impacts (as qmk#6947)

* Remove some dead code and whitespace

* Remove unused code

* Remove unused code

* Update keyboards/zinc/rev1/config.h

* Update keyboards/zinc/reva/config.h

* Update keyboards/zinc/keymaps/default/rules.mk

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/readme_en.md

* Update keyboards/zinc/keymaps/default/readme_en.md

* Breaking Changes Changelog
noroadsleft pushed a commit to noroadsleft/qmk_firmware that referenced this pull request May 28, 2020
* Add Zinc keyboard

* Fix RGB LED init of monks/keymap.c

* Add RGBLED_BOTH_ENABLE option

* Fix RGBLED_BOTH_ENABLE option

* Add LED_BOTH_ENABLE feature to 2 keymaps

* Use split_common instead of own split flies

* Fix split LED

* Fix RGB LED config for iOS device

* Add RGB_MOD reverse key to default keymap

* Update readme_jp.md of default keymap

* Add readme_en.md of default keymap

* Merge branch 'master' into zinc_splitcommon

# Conflicts:
#	keyboards/zinc/serial.c

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/readme.md

* Update keyboards/zinc/rules.mk

* Update keyboards/zinc/rules.mk

* Update toshi0383 keymap

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Update keyboards/zinc/keymaps/toshi0383/rules.mk

* Remove build option firmware size impacts (as qmk#6947)

* Remove some dead code and whitespace

* Remove unused code

* Remove unused code

* Update keyboards/zinc/rev1/config.h

* Update keyboards/zinc/reva/config.h

* Update keyboards/zinc/keymaps/default/rules.mk

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/keymap.c

* Update keyboards/zinc/keymaps/default/readme_en.md

* Update keyboards/zinc/keymaps/default/readme_en.md

* Breaking Changes Changelog
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Update rules.mk template to remove build option size impacts

* Add rules.mk cleaning script

* Update all rules.mk files to remove build option firmware size impact messages

* Remove references to feature filesize in documentation

* Revert "Update all rules.mk files to remove build option firmware size impact messages"

This reverts commit 7cfe709.

* Fix regex in cleanup script and exclude keymaps/ directories

* Update quantum/template/avr/rules.mk

Fixed missing tabs/spaces.

Co-Authored-By: fauxpark <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove build option firmware size impacts from rules.mk
5 participants