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

Re-refactor Mokulua #17125

Merged
merged 27 commits into from
Jul 17, 2022
Merged

Re-refactor Mokulua #17125

merged 27 commits into from
Jul 17, 2022

Conversation

kylemccreery
Copy link
Contributor

@kylemccreery kylemccreery commented May 17, 2022

Description

The refactor of the Mokulua firmware removed the support for one of the 2 possible PCBs that this split board can use. This readds support for the second PCB while keeping a couple of the changes that were suggested by that refactor.

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

  • Readding removed support for PCB

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 via Adds via keymap and/or updates keyboard for via support labels May 17, 2022
@kylemccreery
Copy link
Contributor Author

@waffle87 If you could check this out and see if it is clear what I am trying to do.

Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

Conflicts need to be resolved where the branch was reused from a previous PR.

@kylemccreery
Copy link
Contributor Author

@zvecr Fixed. Thanks. Sorry I missed that.

@kylemccreery kylemccreery requested a review from zvecr May 17, 2022 16:27
@kylemccreery
Copy link
Contributor Author

Because of the error in PR Lint check, would it be better served to have an individal info.json for each of the PCB versions (mirrored and standard)?

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.

It looks like you have changes to the submodules here. These need to be reverted before the PR can be approved.

Rebasing sometimes will fix this. Otherwise, you'd want to do something like this to fix them:

git checkout master
git submodule foreach 'git log -n1 --oneline'
# remember the first abcdef1234 on the submodules that have an issue
git checkout @@your-branch@@
# for each of the problematic repo's:
cd lib/@@name@@
git checkout abcdef1234

keyboards/mechwild/mokulua/mirrored/mirrored.c Outdated Show resolved Hide resolved
keyboards/mechwild/mokulua/standard/standard.c Outdated Show resolved Hide resolved
kylemccreery and others added 2 commits June 13, 2022 16:45
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
@kylemccreery kylemccreery requested a review from drashna June 13, 2022 20:46
keyboards/mechwild/mokulua/mirrored/config.h Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team June 16, 2022 22:02
@kylemccreery kylemccreery requested a review from fauxpark June 16, 2022 22:41
@kylemccreery
Copy link
Contributor Author

@zvecr @fauxpark Anything else that needs to be changed on this for a rereview?

Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Slight __attribute__ ((weak)) ✔️.

I diff'd the previous mirrored/standard implementation and came to the same conclusion @waffle87 seems to have done, but if the keyboard maintainer says some of the supported hardware doesn't work on the post-refactor version, I'm inclined to believe them.

Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Took another look at this and spotted an error in the original refactor that this PR solves.

@noroadsleft noroadsleft merged commit a444ccd into qmk:master Jul 17, 2022
@noroadsleft
Copy link
Member

Thanks!

smocky pushed a commit to smocky/qmk_firmware that referenced this pull request Jul 22, 2022
* allowing the kt60 file to be modified so I can do things while waiting for it to be fixed upstream

* initial commit for mokulua keyboard

* Split the board into mirrored and standard layouts.

* Prepping for PR. Silly keymap added.

* prepped for PR

* Apply suggestions from code review

* Fixing firmware from the refactor that removed the mirrored layout.

* Small tweaks using changes from refactor

* Changed the name of the layouts back to match the original to resolve conflict in info.json

* these files needed to be removed as well, they were added as a part of the refactor

* info.json moveds to be different for each build

* Another file had to be removed and the mirrored.c file changed to call mirrored.h instead of standard.h

* fixing chibios ver

* force deleting to revert

* fixing chibios shit

* Update keyboards/mechwild/mokulua/mirrored/mirrored.c

* Update keyboards/mechwild/mokulua/standard/standard.c

* Removing tabs and replacing with 4 spaces. Small style and formatting changes.
Haruki pushed a commit to Haruki/qmk_firmware that referenced this pull request Jul 23, 2022
* allowing the kt60 file to be modified so I can do things while waiting for it to be fixed upstream

* initial commit for mokulua keyboard

* Split the board into mirrored and standard layouts.

* Prepping for PR. Silly keymap added.

* prepped for PR

* Apply suggestions from code review

* Fixing firmware from the refactor that removed the mirrored layout.

* Small tweaks using changes from refactor

* Changed the name of the layouts back to match the original to resolve conflict in info.json

* these files needed to be removed as well, they were added as a part of the refactor

* info.json moveds to be different for each build

* Another file had to be removed and the mirrored.c file changed to call mirrored.h instead of standard.h

* fixing chibios ver

* force deleting to revert

* fixing chibios shit

* Update keyboards/mechwild/mokulua/mirrored/mirrored.c

* Update keyboards/mechwild/mokulua/standard/standard.c

* Removing tabs and replacing with 4 spaces. Small style and formatting changes.
schattenbrot pushed a commit to schattenbrot/qmk_firmware that referenced this pull request Aug 2, 2022
* allowing the kt60 file to be modified so I can do things while waiting for it to be fixed upstream

* initial commit for mokulua keyboard

* Split the board into mirrored and standard layouts.

* Prepping for PR. Silly keymap added.

* prepped for PR

* Apply suggestions from code review

* Fixing firmware from the refactor that removed the mirrored layout.

* Small tweaks using changes from refactor

* Changed the name of the layouts back to match the original to resolve conflict in info.json

* these files needed to be removed as well, they were added as a part of the refactor

* info.json moveds to be different for each build

* Another file had to be removed and the mirrored.c file changed to call mirrored.h instead of standard.h

* fixing chibios ver

* force deleting to revert

* fixing chibios shit

* Update keyboards/mechwild/mokulua/mirrored/mirrored.c

* Update keyboards/mechwild/mokulua/standard/standard.c

* Removing tabs and replacing with 4 spaces. Small style and formatting changes.
imhoffman pushed a commit to imhoffman/qmk_firmware that referenced this pull request Aug 20, 2022
* allowing the kt60 file to be modified so I can do things while waiting for it to be fixed upstream

* initial commit for mokulua keyboard

* Split the board into mirrored and standard layouts.

* Prepping for PR. Silly keymap added.

* prepped for PR

* Apply suggestions from code review

* Fixing firmware from the refactor that removed the mirrored layout.

* Small tweaks using changes from refactor

* Changed the name of the layouts back to match the original to resolve conflict in info.json

* these files needed to be removed as well, they were added as a part of the refactor

* info.json moveds to be different for each build

* Another file had to be removed and the mirrored.c file changed to call mirrored.h instead of standard.h

* fixing chibios ver

* force deleting to revert

* fixing chibios shit

* Update keyboards/mechwild/mokulua/mirrored/mirrored.c

* Update keyboards/mechwild/mokulua/standard/standard.c

* Removing tabs and replacing with 4 spaces. Small style and formatting changes.
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
* allowing the kt60 file to be modified so I can do things while waiting for it to be fixed upstream

* initial commit for mokulua keyboard

* Split the board into mirrored and standard layouts.

* Prepping for PR. Silly keymap added.

* prepped for PR

* Apply suggestions from code review

* Fixing firmware from the refactor that removed the mirrored layout.

* Small tweaks using changes from refactor

* Changed the name of the layouts back to match the original to resolve conflict in info.json

* these files needed to be removed as well, they were added as a part of the refactor

* info.json moveds to be different for each build

* Another file had to be removed and the mirrored.c file changed to call mirrored.h instead of standard.h

* fixing chibios ver

* force deleting to revert

* fixing chibios shit

* Update keyboards/mechwild/mokulua/mirrored/mirrored.c

* Update keyboards/mechwild/mokulua/standard/standard.c

* Removing tabs and replacing with 4 spaces. Small style and formatting changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap 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