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 NCC1701KB and adding via support #22705

Closed

Conversation

jessel92
Copy link
Contributor

Bringing the NCC up to date with the latest QMK changes, as well as adding a keymap for Via support

Description

Just cleaned up an old keyboard, that needed updating and also add a keymap for Via support

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

DOIO2022 and others added 10 commits December 17, 2023 18:51
* Add files via upload

* Update keyboards/doio/kb3x/config.h

Co-authored-by: Joel Challis <[email protected]>

* Update keyboards/doio/kb3x/info.json

Co-authored-by: Joel Challis <[email protected]>

* Update keyboards/doio/kb3x/keymaps/via/keymap.c

Co-authored-by: Joel Challis <[email protected]>

* Update keyboards/doio/kb3x/config.h

Co-authored-by: adophoxia <[email protected]>

* Update keyboards/doio/kb3x/keymaps/default/keymap.c

Co-authored-by: Joel Challis <[email protected]>

* Update rules.mk

* Update info.json

* Update keyboards/doio/kb3x/info.json

Co-authored-by: Duncan Sutherland <[email protected]>

* Update keyboards/doio/kb3x/rules.mk

Co-authored-by: adophoxia <[email protected]>

* Add files via upload

* Update keyboards/doio/kb3x/info.json

Co-authored-by: Duncan Sutherland <[email protected]>

* Update keyboards/doio/kb3x/config.h

Co-authored-by: Duncan Sutherland <[email protected]>

* Delete keyboards/doio/kb3x/rules.mk

* Update keyboards/doio/kb3x/keymaps/default/keymap.c

Co-authored-by: Drashna Jaelre <[email protected]>

* Update keyboards/doio/kb3x/keymaps/via/keymap.c

Co-authored-by: Drashna Jaelre <[email protected]>

* Add KB19

* Delete keyboards/doio/kb3x directory

* format  info.json

* format info.json

* Create rules.mk

* Update rules.mk

* Update keyboards/doio/kb19/config.h

Co-authored-by: Ryan <[email protected]>

* Update info.json

* Update keymap.c

* Update keymap.c

* Delete keyboards/doio/kb19/config.h

* Update keyboards/doio/kb19/keymaps/via/keymap.c

Co-authored-by: Drashna Jaelre <[email protected]>

* Update keyboards/doio/kb19/keymaps/default/keymap.c

Co-authored-by: Drashna Jaelre <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
Co-authored-by: adophoxia <[email protected]>
Co-authored-by: Duncan Sutherland <[email protected]>
Co-authored-by: Drashna Jaelre <[email protected]>
Co-authored-by: Ryan <[email protected]>
Bringing the NCC up to date with the latest QMK changes, as well as adding a keymap for Via support
@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Dec 18, 2023
@fauxpark fauxpark requested a review from a team December 18, 2023 23:47
keyboards/themadnoodle/ncc1701kb/v2/keymaps/via/rules.mk Outdated Show resolved Hide resolved
keyboards/themadnoodle/ncc1701kb/v2/info.json Outdated Show resolved Hide resolved
keyboards/themadnoodle/ncc1701kb/v2/info.json Show resolved Hide resolved
keyboards/themadnoodle/ncc1701kb/v2/config.h Outdated Show resolved Hide resolved
keyboards/themadnoodle/ncc1701kb/v2/rules.mk Outdated Show resolved Hide resolved
Deemen17 and others added 4 commits December 18, 2023 17:02
* Add de60fs

* Update

* Add image

* Add line break for rules.mk

* Update keyboards/deemen17/de60fs/keymaps/default/keymap.c

Co-authored-by: Less/Rikki <[email protected]>

* Update keyboards/deemen17/de60fs/info.json

Co-authored-by: Less/Rikki <[email protected]>

* Remove readme of keymap folder

* Change to KC_RSFT

* Add Community Layout support

Co-authored-by: Duncan Sutherland <[email protected]>

* Update keyboards/deemen17/de60fs/info.json

Co-authored-by: Duncan Sutherland <[email protected]>

* Update keyboards/deemen17/de60fs/keymaps/default/keymap.c

Co-authored-by: Duncan Sutherland <[email protected]>

* Update keyboards/deemen17/de60fs/keymaps/via/keymap.c

Co-authored-by: Duncan Sutherland <[email protected]>

* Add description in readme

---------

Co-authored-by: Less/Rikki <[email protected]>
Co-authored-by: Duncan Sutherland <[email protected]>
@jessel92 jessel92 requested a review from zvecr December 19, 2023 01:30
Copy link
Contributor Author

@jessel92 jessel92 left a comment

Choose a reason for hiding this comment

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

Everything should be good to go now! Let me know if this is all correct!

@jessel92 jessel92 changed the base branch from master to develop December 20, 2023 02:47
@jessel92 jessel92 requested a review from fauxpark December 20, 2023 02:48
@jessel92
Copy link
Contributor Author

Keyboard updates need to go through develop.

@fauxpark changed the base branch to develop. I didn't think this needed to go through the develop branch since no files or directories changed names. Let me know if this works. I'm trying to get these through ASAP.

@fauxpark
Copy link
Member

fauxpark commented Dec 20, 2023

It's not just about file and folder names - since master gets merged into develop automatically there is a potential for merge conflicts if the same files are modified in develop. So really only new keyboard additions and urgent bugfixes can go to master.

You have some merge conflicts due to the merge commit you just made. This is not necessary because of the above. Please remove it.

@jessel92 jessel92 changed the base branch from develop to master December 20, 2023 03:09
@jessel92 jessel92 changed the base branch from master to develop December 20, 2023 03:11
@jessel92
Copy link
Contributor Author

It's not just about file and folder names - since master gets merged into develop automatically there is a potential for merge conflicts if the same files are modified in develop. So really only new keyboard additions and urgent bugfixes can go to master.

You have some merge conflicts due to the merge commit you just made. This is not necessary because of the above. Please remove it.

@fauxpark Oh, ok I understand! But the merge conflicts are another story, should I switch back to the master branch? Because it says only those with write access can "resolve conflicts"? what should I do to fix this?

@fauxpark
Copy link
Member

You just need to undo the last commit.

@jessel92
Copy link
Contributor Author

@fauxpark the last commit was a readme.md commit. or are you talking about switching branches?

@fauxpark
Copy link
Member

image

@jessel92
Copy link
Contributor Author

image

@fauxpark ok o reverted it, but it now looks like i have a "python" tag also this a ton of conflcs

@jessel92
Copy link
Contributor Author

@fauxpark it seems like changing the base branch of the PR REALLY messed things up.

@jessel92 jessel92 changed the base branch from develop to master December 20, 2023 03:34
@fauxpark fauxpark changed the base branch from master to develop December 20, 2023 03:42
@fauxpark
Copy link
Member

fauxpark commented Dec 20, 2023

No, there must be something wrong with your branch. Although it doesn't seem to have diverged, it's only 14 commits ahead of master.

It might be easier to just resubmit this PR pointing at develop.

@jessel92
Copy link
Contributor Author

@fauxpark Yes, it was totally fine before the PRs base branch change. If I open a new a PR is there any way that we can push it through so I don't have to go through all that again?

jessel92 added a commit to The-Mad-Noodle/qmk_firmware that referenced this pull request Dec 20, 2023
After a problem with the last PR qmk#22705 this is a retry WITH all of the corrections and commits suggested by reviewers
jessel92 added a commit to The-Mad-Noodle/qmk_firmware that referenced this pull request Dec 20, 2023
RETRY of PR qmk#22705 After a strange problem with the develop branch. It was causing a HUGE amount of conflict errors. I was told to just make a new PR. This contains ALL of the suggestions and commits from the reviewers of the form PR, so there shouldn't be any changes.
@jessel92
Copy link
Contributor Author

@fauxpark Even when i try to create a new branch on my fork and make a new PR to the develop branch, I get HUNDREDs of commits that are not mine. What's going on? Is this an issue on my end or yours? Im just trying to clean up my keymaps and make an old keypad fit the new QMK json style.

@fauxpark
Copy link
Member

I'm going to close this PR. Please resubmit it properly.

@fauxpark fauxpark closed this Dec 20, 2023
@fauxpark
Copy link
Member

fauxpark commented Dec 20, 2023

Looks like the master->develop merged failed due to conflicts (related to the ongoing effort to remove user keymaps, apparently those PRs are going to master for some reason). So that explains what all the mess here is about. I've fixed it up; develop should now contain all of master's history.

jessel92 added a commit to The-Mad-Noodle/qmk_firmware that referenced this pull request Dec 20, 2023
RETRY of PR qmk#22705 After a strange problem with the develop branch. It was causing a HUGE amount of conflict errors. I was told to just make a new PR. This contains ALL of the suggestions and commits from the reviewers of the original PR, so there shouldn't be any changes.
@jessel92
Copy link
Contributor Author

Looks like the master->develop merged failed due to conflicts (related to the ongoing effort to remove user keymaps, apparently those PRs are going to master for some reason). So that explains what all the mess here is about. I've fixed it up; develop should now contain all of master's history.

@fauxpark Ugh that's rough, but thank you so much for helping through all the hassle! The new PR was a success! I have a few other PRs open right now that I think you're reviewing as well. I hope they don't cause as much trouble and we can get them done quickly! I think you're up for review on Noodlepad Additions and Updates #22701 ;-)

@jessel92 jessel92 deleted the ncc1701kb-updating-and-via-support branch December 27, 2023 04:25
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.

9 participants