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 custom user code #22485

Closed
wants to merge 11 commits into from
Closed

Conversation

waffle87
Copy link
Member

Description

Removes users/*, layouts/community/*, and user/non-default(ish) keymaps in an effort to reduce unmaintained code throughout the repository.

Types of Changes

  • Keymap/layout/userspace (addition or update)

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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).

@waffle87 waffle87 added breaking_change_2023q4 no-ci Signals to the CI runners not to build. labels Nov 16, 2023
@github-actions github-actions bot added keymap python and removed no-ci Signals to the CI runners not to build. labels Nov 16, 2023
@tzarc tzarc added the no-ci Signals to the CI runners not to build. label Nov 16, 2023
dunk2k added a commit to dunk2k/qmk_firmware that referenced this pull request Nov 17, 2023
dunk2k added a commit to dunk2k/qmk_firmware that referenced this pull request Nov 17, 2023
dunk2k added a commit to dunk2k/qmk_firmware that referenced this pull request Nov 17, 2023
dunk2k added a commit to dunk2k/qmk_firmware that referenced this pull request Nov 17, 2023
@github-actions github-actions bot added the via Adds via keymap and/or updates keyboard for via support label Nov 17, 2023
@github-actions github-actions bot removed the via Adds via keymap and/or updates keyboard for via support label Nov 18, 2023
@waffle87 waffle87 marked this pull request as ready for review November 18, 2023 20:40
@waffle87 waffle87 requested a review from a team November 18, 2023 20:42
@drashna
Copy link
Member

drashna commented Nov 19, 2023

We should have a discord command (as well as an FAQ entry) ready to go before this gets merged/hits master.

@daskygit
Copy link
Member

Are we not keeping vendor keycaps?

@tzarc
Copy link
Member

tzarc commented Nov 21, 2023

Are we not keeping vendor keycaps?

You probably meant keymaps :P


@waffle87 -- given the fact that there are a bunch of default-ish or vendor keymaps deleted, we've decided we're going to approach the removal slightly differently.

  • We'll produce a full list of keymap names across the entire repository, either by maintaining a GDoc or a separate branch
  • Amongst ourselves determine which keymap names are to be kept
  • Create a script that can remove only the listed set of keymaps
  • Perform the deletion en-masse

The aim is to still do this for the November 2023 merge date -- further discussion will be on Discord.

@thomasbaart
Copy link
Contributor

thomasbaart commented Nov 21, 2023

Hi! I noticed our debug keymaps are included in this removal, and I'd like to kindly request to have those exempt, as we refer to them in our build guides and depend on their presence during our firmware build process (which get presented at https://firmware.splitkb.com).

If this is undesirable, please do let me know as well so I can see if we can work around that in a different way. Thank you!

@waffle87 waffle87 closed this Nov 22, 2023
@mmccoyd
Copy link
Contributor

mmccoyd commented Nov 22, 2023

Can I vote for keeping the hillside/xx/default_dot_c keymaps, as the hillside's creator? Providing defaults of .json and .c seemed desirable. If not, I can reexamine having a default.json, given that vial exists.
Thanks!

@tzarc
Copy link
Member

tzarc commented Nov 23, 2023

Can I vote for keeping the hillside/xx/default_dot_c keymaps, as the hillside's creator? Providing defaults of .json and .c seemed desirable. If not, I can reexamine having a default.json, given that vial exists. Thanks!

Under normal circumstances, people can migrate from json-based to C-based using qmk json2c. No generalised documentation exists pointing people at anything like default_dot_c, and qmk new-keymap will duplicate default anyway.

Other than C vs. JSON, is there anything else in particular as to why you're advocating for keeping both?

@mmccoyd
Copy link
Contributor

mmccoyd commented Nov 27, 2023

Other than C vs. JSON, is there anything else in particular as to why you're advocating for keeping both?

Nothing more. Guess I'm not up to date on the conversions.
Thanks.

dunk2k added a commit to dunk2k/qmk_firmware that referenced this pull request Dec 4, 2023
@waffle87 waffle87 deleted the remove_user_keymap branch December 5, 2023 22:59
dunk2k added a commit to dunk2k/qmk_firmware that referenced this pull request Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap no-ci Signals to the CI runners not to build. python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants