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] jacky_studio/piggy60 #18012

Merged
merged 1 commit into from
Aug 14, 2022
Merged

[keyboard] jacky_studio/piggy60 #18012

merged 1 commit into from
Aug 14, 2022

Conversation

lesshonor
Copy link
Contributor

Description

Adds Jacky's Piggy60 to QMK.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

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 Aug 12, 2022
keyboards/jacky_studio/piggy60/config.h Show resolved Hide resolved
keyboards/jacky_studio/piggy60/readme.md Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team August 13, 2022 02:23

#pragma once

#define LED_PIN_ON_STATE 0
Copy link
Member

Choose a reason for hiding this comment

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

Also, just a heads up, you can use "on_state" as true/false under "indicators" in the json file to set this.

Copy link
Contributor Author

@lesshonor lesshonor Aug 13, 2022

Choose a reason for hiding this comment

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

I was wondering how I missed this, and the answer is that it's not in master.
Will this PR be held until after the next breaking changes merge? I'll admit I was rushing to make it in under the wire, but if that's not gonna work out, I'll make that change. was pretty disappointing to have to add the config header for one define lmao

probably also worth noting; this feature isn't documented yet either. (I wouldn't mind submitting a PR for that if I can but as I understand it, develop is closed for business...?)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, no worries, then.

And if it helps, develop is scheduled to be merged into master at the end of the month (in 2 weeks). Also, qmk.tzarc.io compiles firmware images from develop.

@drashna drashna requested a review from a team August 13, 2022 04:50
Copy link
Contributor

@keyboard-magpie keyboard-magpie left a comment

Choose a reason for hiding this comment

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

Approving in current state, but noting the conversation had regarding indicators

@drashna drashna merged commit ca2d05f into qmk:master Aug 14, 2022
@lesshonor lesshonor deleted the piggy60 branch August 14, 2022 00:50
imhoffman pushed a commit to imhoffman/qmk_firmware that referenced this pull request Aug 20, 2022
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
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.

3 participants