Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

cmake: Require nlohmann-json when building the front-end #914

Merged
merged 1 commit into from
Sep 6, 2022
Merged

cmake: Require nlohmann-json when building the front-end #914

merged 1 commit into from
Sep 6, 2022

Conversation

rmnvgr
Copy link
Contributor

@rmnvgr rmnvgr commented Sep 5, 2022

Explain the Pull Request

Nlohmann-json is used in the about dialog, yet wasn't required when building the front-end, leading to failing compilation. This ensures that it is correctly required. See flathub/flathub#3159 (comment).

Completion Checklist

  • I have added myself to the Copyright and License headers and files.
  • I will maintain this code in the future and have added myself to CODEOWNERS. <-- I haven't added myself to CODEOWNERS, do I need to for such a simple patch?
  • I have tested this change on the following platforms:
    • MacOS 10.15
    • MacOS 11
    • MacOS 12
    • Ubuntu 20.04
    • Ubuntu 22.04
    • Windows 10
    • Windows 11

@rmnvgr rmnvgr requested a review from Xaymar as a code owner September 5, 2022 07:52
@Xaymar
Copy link
Owner

Xaymar commented Sep 5, 2022

LGTM. Will squash to remove the manual new lines in the message though, unless you want to fix those yourself?

I haven't added myself to CODEOWNERS, do I need to for such a simple patch?

Nope, it's perfectly fine to submit patches without adding yourself to coode owners. This is more of a requirement for feature additions

Nlohmann-json is used in the about dialog, yet wasn't required when building the front-end, leading to failing compilation. This ensures that it is correctly required.
@rmnvgr
Copy link
Contributor Author

rmnvgr commented Sep 5, 2022

I have removed the manual new lines.

@Xaymar Xaymar merged commit ab91778 into Xaymar:master Sep 6, 2022
@Xaymar
Copy link
Owner

Xaymar commented Sep 6, 2022

May have actually squashed instead of rebasing as intended. Oh well.

@rmnvgr rmnvgr deleted the cmake-frontend-json branch September 6, 2022 07:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants