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

Updating the noodlepad_micro keymaps with custom keycodes #23003

Closed

Conversation

jessel92
Copy link
Contributor

@jessel92 jessel92 commented Feb 2, 2024

Description

I added some custom keycodes to the noodlepad_micro keymaps. This also required me to modify the config,h file with the features implemented in the keycodes.

Forgive me if there is json version of some of the config.h options I enabled, I tried some and they didn't work, and I'm not really sure where to find if there is. Let me know!

Also, I'm sure the keycodes code is a little messy and inefficient, but they work great hahaha

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

@jessel92
Copy link
Contributor Author

Any movement on this?

@jessel92
Copy link
Contributor Author

jessel92 commented Mar 20, 2024

@drashna

EDIT: Disregard this, I only get this error after I run the qmk info -f json -kb themadnoodle/noodlepad_micro command

I'm also now getting THIS error that I was not getting before when I try to compile my keymap
Did something change in the master branch since I opened this PR that would cause this issue?

/RP2040/ws2812_vendor.c:4:
drivers/ws2812.h:60:30: error: 'RGBLIGHT_LED_COUNT' undeclared here (not in a function); did you mean 'RGBLIGHT_LAYER_BLINK'?
   60 | #    define WS2812_LED_COUNT RGBLIGHT_LED_COUNT
      |                              ^~~~~~~~~~~~~~~~~~
platforms/chibios/drivers/vendor/RP/RP2040/ws2812_vendor.c:138:46: note: in expansion of macro 'WS2812_LED_COUNT'
  138 | static uint32_t                WS2812_BUFFER[WS2812_LED_COUNT];
      |                                              ^~~~~~~~~~~~~~~~
platforms/chibios/drivers/vendor/RP/RP2040/ws2812_vendor.c:138:32: error: 'WS2812_BUFFER' defined but not used [-Werror=unused-variable]
  138 | static uint32_t                WS2812_BUFFER[WS2812_LED_COUNT];
      |                                ^~~~~~~~~~~~~
cc1.exe: all warnings being treated as errors
 [ERRORS]
 |
 |
 |
make: *** [builddefs/common_rules.mk:373: .build/obj_themadnoodle_noodlepad_micro_via/ws2812_vendor.o] Error 1

@jessel92 jessel92 requested a review from drashna March 20, 2024 20:40
@jessel92
Copy link
Contributor Author

@drashna Please let me know if we can get this merged as soon as possible. Because I have a PR with VIA that predates these custom codes, and they kindly agreed to wait for these changes to be merged before proceeding with their PR.

@jessel92
Copy link
Contributor Author

jessel92 commented Apr 4, 2024

@drashna, I see everything was approved, but the PR was never merged. Is there any way we can get this merged?

@waffle87
Copy link
Member

waffle87 commented Apr 4, 2024

At least two review approvals are needed before merge.

Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

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

Keyboard updates need to go through develop.

@waffle87 waffle87 changed the base branch from master to develop April 4, 2024 00:51
@waffle87
Copy link
Member

waffle87 commented Apr 4, 2024

Also, see merge conflicts to be resolved.

@jessel92
Copy link
Contributor Author

jessel92 commented Apr 4, 2024

At least two review approvals are needed before merge.

@waffle87 Ahh, I see ok, thanks for the info.
@fauxpark This is a keymap update, isn't it? So shouldn't it be able to be merged straight into master??

@fauxpark
Copy link
Member

fauxpark commented Apr 4, 2024

There are changes to keyboard-level files.

@jessel92
Copy link
Contributor Author

jessel92 commented Apr 4, 2024

There are changes to keyboard-level files.

@fauxpark So, does that mean I need to wait until May for it to be merged into the master branch from develop, just because I added a few config changes?

Also, what's this branch conflict about? It wasn't there a few minutes ago, and how do I resolve it?

@waffle87
Copy link
Member

waffle87 commented Apr 4, 2024

does that mean I need to wait until May for it to be merged into the master branch from develop

Yes. The merge conflicts are product of switching the target branch. See this doc.

@jessel92 jessel92 force-pushed the Micro_Custom-Features-Keymaps branch from ea92753 to 50d9dcf Compare April 4, 2024 01:33
I think
@jessel92 jessel92 requested review from fauxpark and waffle87 April 4, 2024 01:39
@jessel92
Copy link
Contributor Author

jessel92 commented Apr 4, 2024

@fauxpark @waffle87 ok, I think I was able to resolve it all. Correct? I rebased to develop, then recommitted the changes made in the master branch.

Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label May 20, 2024
@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label May 21, 2024
@jessel92 jessel92 requested a review from fauxpark May 21, 2024 15:30
Copy link

github-actions bot commented Jul 6, 2024

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Jul 6, 2024
@jessel92
Copy link
Contributor Author

jessel92 commented Jul 7, 2024

@fauxpark @waffle87 @drashna

This PR has been open since February 2, meaning it has now been pending for six months. I have promptly addressed every requested change within hours of the request. However, GitHub has attempted to mark this as stale twice, even though I am merely waiting for the next steps from everyone involved. Considering that this now ALSO needs to go through the develop branch before hitting the master, I would really appreciate it if we could resolve this as soon as possible (unless there is a way for us to get this merged directly to the master branch).

I am uncertain what else I need to do.

Additionally, due to the extremely long wait time, this PR has become somewhat outdated. I have new changes that need to be added once this is merged, along with a new keypad PR that I can't open until this is merged.

I am trying to remain patient, understanding that you all have a lot on your plate and that open-source work can be challenging. However, this prolonged process has significantly thrown a wrench in my business at this point.

@fauxpark fauxpark requested a review from a team July 7, 2024 07:56
@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Jul 8, 2024
@tzarc
Copy link
Member

tzarc commented Aug 26, 2024

As of August 26, 2024, qmk/qmk_firmware is no longer accepting VIA-enabled keymaps as these have now transitioned to a repository under the VIA team's control.

As you've submitted a PR containing via or VIA-enabled keymap(s), this is your notice that they should be removed from this PR. You should now submit a secondary PR to the VIA QMK Userspace repository with your associated via or VIA-enabled keymaps instead.

Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Oct 11, 2024
Copy link

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]

@github-actions github-actions bot closed this Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap stale Issues or pull requests that have become inactive without resolution. 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