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.

Sign up for GitHub

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

Jump to bottom

bastardkb: fix info.json changes that got reverted during the last merge from master to develop #17800

Merged
merged 2 commits into from
Jul 26, 2022
Merged

bastardkb: fix info.json changes that got reverted during the last merge from master to develop #17800

merged 2 commits into from
Jul 26, 2022

Conversation

0xcharly
Copy link
Contributor

Fix forward changes that were reverted during the last merge.

cc @drashna

Description

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

@drashna drashna requested a review from a team July 26, 2022 04:27
@fauxpark
Copy link
Member

Are you sure that this works? I've been deliberately avoiding keyboards split into revisions for #17782 etc. because there is an issue with the order in which the CLI merges info.json's together.

@0xcharly
Copy link
Contributor Author

0xcharly commented Jul 26, 2022 •
edited
Loading

This seems to work for all these boards and configurations without any issue (all keyboards under keyboards/bastardkb, including the Charybdis that is already using this setup with multiple level of info.json, eg. https://github.com/qmk/qmk_firmware/blob/develop/keyboards/bastardkb/charybdis/3x5/v2/elitec/info.json).

If this is not the intended use, I'll revert those changes and update all the info.json files under bastardkb.

@0xcharly 0xcharly force-pushed the upstream-info-json branch from 5d468c7 to 71e97ac Compare July 26, 2022 05:12
@fauxpark
Copy link
Member

I think it should be fine if each key is only specified once - for example if an info.json under rev1 "overrides" the manufacturer string in the top-level info.json, the latter is used instead, though it should be the other way around. But I have not really looked into this yet, so I've just been putting it off in these PRs. If the generated config.h has the right values, no problem.

@0xcharly
Copy link
Contributor Author

You're correct, values cannot be overridden, only combined. The resulting config.h looks correct to me, and all configurations build correctly with these changes.

@keyboard-magpie keyboard-magpie merged commit 2444506 into qmk:develop Jul 26, 2022
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
@0xcharly @nolanseaton
bastardkb: fix info.json changes that got reverted during the last me…
5d7c401 
…rge from `master` to `develop` (qmk#17800)

* bastardkb: fix info.json changes that got reverted during the last merge from `master` to `develop`

* Remove board name from root folder `info.json`
@0xcharly 0xcharly deleted the upstream-info-json branch April 7, 2024 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keyboard-magpie keyboard-magpie keyboard-magpie approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@0xcharly @fauxpark @drashna @keyboard-magpie