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] Add Arrow Mechanics WINGS #23227

Closed
wants to merge 32 commits into from

Conversation

arrowmechanics
Copy link
Contributor

@arrowmechanics arrowmechanics commented Mar 6, 2024

Description

Add base support for Arrow Mechanics WINGS keyboard with all the essential features

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

keyboards/arrowmechanics/wings/info.json Outdated Show resolved Hide resolved
keyboards/arrowmechanics/wings/rules.mk Outdated Show resolved Hide resolved
keyboards/arrowmechanics/wings/info.json Show resolved Hide resolved
Copy link
Contributor Author

@arrowmechanics arrowmechanics left a comment

Choose a reason for hiding this comment

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

applied review suggestions, please see one comment

Copy link
Contributor Author

@arrowmechanics arrowmechanics left a comment

Choose a reason for hiding this comment

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

applied all review suggestions

@arrowmechanics arrowmechanics changed the title Add keyboard: Arrow Mechanics WINGS [Keyboard] Add Arrow Mechanics WINGS Mar 8, 2024
@arrowmechanics
Copy link
Contributor Author

No more functional changes in this PR! (just fixes if needed)

@arrowmechanics arrowmechanics marked this pull request as draft March 8, 2024 23:01
@arrowmechanics arrowmechanics marked this pull request as ready for review March 8, 2024 23:01
keyboards/arrowmechanics/wings/config.h Outdated Show resolved Hide resolved
keyboards/arrowmechanics/wings/config.h Outdated Show resolved Hide resolved
@zvecr
Copy link
Member

zvecr commented Mar 14, 2024

image

All the constant merging is doing is burning CI minutes and making other PRs workflow runs take longer.

@arrowmechanics
Copy link
Contributor Author

arrowmechanics commented Mar 14, 2024 via email

@arrowmechanics
Copy link
Contributor Author

Is there anything else that stops us from merging?

@arrowmechanics
Copy link
Contributor Author

arrowmechanics commented Mar 14, 2024

All the constant merging is doing is burning CI minutes and making other PRs workflow runs take longer.

Also, this might be a thing for thought when it comes to maintenance rules of the QMK repo - how should I know as a minor contributor, how many updates to the main branch should I expect before my PR is taken care of?

I understand my mistake now, but it's not that obvious in general for there are no releases, no time-related or anyhow strict rules of merging to main. It actually looks confusing as multiple minor updates were merged in the meantime without even passing the pipelines - maybe it's a good idea to elaborate on that in the docs?
Or maybe I shouldnt be allowed to be so over-eager with triggering the CI? Isn't there a maintainer-validation mechanism?

@tzarc tzarc added the no-ci Signals to the CI runners not to build. label Mar 14, 2024
@tzarc
Copy link
Member

tzarc commented Mar 14, 2024

It's not obvious, because the vast majority of people aren't so eager to update their repo.
I run QMK CI locally, so I pay for electricity costs associated with it -- normal use I'm okay with.

I've disabled CI for your PR for now. If/when collabs get back to it, they can remove the label and retrigger CI on demand.

@arrowmechanics
Copy link
Contributor Author

It's not obvious, because the vast majority of people aren't so eager to update their repo. I run QMK CI locally, so I pay for electricity costs associated with it -- normal use I'm okay with.

I've disabled CI for your PR for now. If/when collabs get back to it, they can remove the label and retrigger CI on demand.

I really am keen, we're yet another start-up that took up QMK as a key element. Still I didn't know You run it locally.
This makes me think it might be good time to start doing this:
image
I promise to go for more as soon as we're profitable :P

keyboards/arrowmechanics/wings/config.h Outdated Show resolved Hide resolved
keyboards/arrowmechanics/wings/readme.md Outdated Show resolved Hide resolved
Co-authored-by: Drashna Jaelre <[email protected]>
@arrowmechanics
Copy link
Contributor Author

This PR seems to have too many comments/issues vs. few changes that it actually introduces. Closing to create a clean version since it may take too much time to dig into things that are already fixed.

@philvec philvec deleted the arrowmechanics_wings branch March 21, 2024 19:17
@philvec philvec mentioned this pull request Mar 21, 2024
14 tasks
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants