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

Revert "Update build instructions and docker file to download submodules" #5533

Merged
merged 3 commits into from
Apr 10, 2019

Conversation

vomindoraan
Copy link
Contributor

@vomindoraan vomindoraan commented Apr 2, 2019

Reverts #2724

The changes that PR made to the dockerfile and build script are not correct. make clean and make git-submodule should not be executed every time the container is run, as this can slow down keymap build times significantly. Submodules should already be present in the directory when the image is built. CI enforces this for official images, and you're supposed to have the submodules cloned ahead of time if you're building a custom image locally.

Furthermore, the change to the docs is unnecessary as well, since the guide instructs the user to use git clone --recurse-submodules to clone the repo (as can be seen here and here), which also clones the submodules.

However, after discussing this with the original PR's author, I've decided that it's a good idea to keep a reminder to run make git-submodule in the build tools docs.

For more details, see the discussion in the original PR.

@metaflow
Copy link
Contributor

metaflow commented Apr 2, 2019 via email

Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

Looks good

@mechmerlin mechmerlin merged commit 4882f7f into qmk:master Apr 10, 2019
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Apr 14, 2019
* 'master' of https://github.com/qmk/qmk_firmware: (101 commits)
  Change table names
  Cleanup code
  Make send_char multilined for readaability
  Clean up some of the code
  Add AltGr/RALT support to Send String
  Add vendor and product arguments to atmega32a_program.py - fix hex print
  Add vendor and product arguments to atmega32a_program.py
  Max out range for max compatibility
  Fix ATSAM's USB HID descriptor to support higher heycodes
  Fix build detection of keymap and board changes
  Apply MAKEFLAGS to speed up CI
  Flesh out keymaps and some tweaks to indicators (qmk#5608)
  add in the missing #defines
  Update ps2avrgb readme template (qmk#5584)
  Use DIRECT_PINS for BDN9 (qmk#5603)
  Port DIRECT_PINS from split_common/matrix.c to matrix.c (qmk#5091)
  Keyboard: Add yosino58 Keyboard (qmk#5465)
  fix offset in matrix (qmk#5600)
  Remove caveat about combos with shared keys (qmk#5601)
  Revert "Update build instructions and docker file to download submodules" (qmk#5533)
  ...
shimesaba-type0 pushed a commit to shimesaba-type0/qmk_firmware that referenced this pull request Jun 22, 2019
…les" (qmk#5533)

* Revert "Update build instructions and Dockerfile to download submodules"

This reverts commit 9321054.

* Update build tools docs based on feedback

* Fix bad link in build tools docs
Timbus pushed a commit to Timbus/qmk_firmware that referenced this pull request Jun 23, 2019
…les" (qmk#5533)

* Revert "Update build instructions and Dockerfile to download submodules"

This reverts commit 9321054.

* Update build tools docs based on feedback

* Fix bad link in build tools docs
@vomindoraan vomindoraan deleted the revert-2724-patch-1 branch May 7, 2020 14:23
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