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

[Keyboard] Move Hillside out of handwired #18751

Merged
merged 5 commits into from
Nov 20, 2022
Merged

Conversation

mmccoyd
Copy link
Contributor

@mmccoyd mmccoyd commented Oct 17, 2022

Description

Move Hillside keyboard family out of handwired as suggested by keyboard-magpie.

Context: Hillside started in handwired, despite being PCB based, based on an old organization standard for non-commercial boards.

The keymaps included by user manna-harbour_miryoku break because they still include RESET.

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

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).

@github-actions github-actions bot added keyboard keymap python via Adds via keymap and/or updates keyboard for via support labels Oct 17, 2022
@mmccoyd mmccoyd marked this pull request as ready for review October 17, 2022 06:59
@keyboard-magpie keyboard-magpie dismissed their stale review October 17, 2022 09:03

cos i'm tired and can't read CI properly

@mmccoyd mmccoyd marked this pull request as draft October 18, 2022 03:53
@mmccoyd mmccoyd marked this pull request as ready for review October 19, 2022 16:47
@mmccoyd
Copy link
Contributor Author

mmccoyd commented Oct 19, 2022

Are the remaining CI fails a CI bug? It builds fine locally. Discord suggests CI still has issues.

@mmccoyd mmccoyd marked this pull request as draft October 24, 2022 18:02
@mmccoyd mmccoyd marked this pull request as ready for review October 24, 2022 18:16
@mmccoyd mmccoyd marked this pull request as draft October 28, 2022 00:08
@mmccoyd mmccoyd marked this pull request as ready for review October 28, 2022 01:48
@drashna
Copy link
Member

drashna commented Oct 31, 2022

Yeah, there is some misconfiguration here, as it shouldn't be attempting to compile "keymaps", as if it was a keymap folder.

@mmccoyd
Copy link
Contributor Author

mmccoyd commented Nov 16, 2022

Yeah, there is some misconfiguration here, as it shouldn't be attempting to compile "keymaps", as if it was a keymap folder.

That does not seem something I can fix, if the QMK CI is reporting issues that do not exist.

@sigprof
Copy link
Contributor

sigprof commented Nov 16, 2022

The problem is this part of the CI code:
https://github.com/qmk/qmk_ci_executor/blob/88605f9607f8554535c8b8d168f9797ce950b3a0/runner/app/ci_worker.py#L288-L300
together with the presence of keyboards/hillside/48/keymaps/json2hill48.py in this PR.

The CI code mistakenly parses the keyboards/hillside/48/keymaps/json2hill48.py path and infers keymap="keymaps", possible_keyboard="hillside" from it, then that nonexistent keymap gets added for all hillside/* keyboards.

@mmccoyd
Copy link
Contributor Author

mmccoyd commented Nov 16, 2022

The CI code mistakenly parses the keyboards/hillside/48/keymaps/json2hill48.py path and infers keymap="keymaps",

Thanks! I can remove the .py so the config issue isn't triggered.

- Add configurator link in 46 readme.
@drashna drashna requested a review from a team November 20, 2022 05:25
@drashna drashna merged commit 543a863 into qmk:develop Nov 20, 2022
@mmccoyd mmccoyd deleted the hillside_move branch November 21, 2022 21:38
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
Co-authored-by: mmccoyd <mmccoyd@cs.berkley.edu>
elpekenin pushed a commit to elpekenin/qmk_firmware that referenced this pull request Dec 7, 2022
Co-authored-by: mmccoyd <mmccoyd@cs.berkley.edu>
crembz pushed a commit to crembz/qmk_firmware that referenced this pull request Dec 18, 2022
Co-authored-by: mmccoyd <mmccoyd@cs.berkley.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap python via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants