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

riot-web conf override broken #81416

Closed
mguentner opened this issue Feb 29, 2020 · 7 comments · Fixed by #81473
Closed

riot-web conf override broken #81416

mguentner opened this issue Feb 29, 2020 · 7 comments · Fixed by #81473
Labels
0.kind: bug
Milestone
20.03

Comments

@mguentner
Copy link
Contributor

Describe the bug

Since f18d4e3 overriding riot-web with a string is no longer possible as it expects an attrset now.

To Reproduce
Steps to reproduce the behavior:

have pkgs.riot-web.override { conf = ''{ "foo": "bar" }'' }; in a config

Expected behavior
overriding with strings is still possible

@pacien
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Mar 1, 2020 •
edited

I think the change intentionally broke compatibility; maybe this should not have been backported to stable. In any case you should rewrite the conf option in Nix or if you can't call builtins.fromJSON on it.

@mguentner
Copy link
Contributor Author

I liked the fact that I could simply copy/paste JSON into my config and edit it directly.

@worldofpeace
Copy link
Contributor

I think the change intentionally broke compatibility; maybe this should not have been backported to stable. In any case you should rewrite the conf option in Nix or if you can't call builtins.fromJSON on it.

Ohh, that's right. I wish I looked at things a bit closer before backporting that to 19.093
That's a non backwards compatible change. Plus, people that switch to a version with this will have no warning or ways to figure it out.

pacien added a commit to pacien/nixpkgs that referenced this issue Mar 1, 2020
@pacien
riot-web: accept conf override as attrset and str
7d2e1a9 
c9e5cca changed the type from str to attrset, which broke some people's config.

GitHub: closes NixOS#81416
@pacien pacien mentioned this issue Mar 1, 2020
Closed
10 tasks
@mguentner
Copy link
Contributor Author

the conf override is also broken for attrset itself, please check #81452

@pacien
Copy link
Contributor

pacien commented Mar 1, 2020

the conf override is also broken for attrset itself, please check #81452

It's not. See #81452 (comment).

pacien added a commit to pacien/nixpkgs that referenced this issue Mar 1, 2020
@pacien
riot-web: accept conf override as attrset and str
ad55e6e 
f18d4e3 changed the type from str to attrset, which broke some people's config.

GitHub: closes NixOS#81416
@pacien pacien mentioned this issue Mar 1, 2020
Merged
10 tasks
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Mar 1, 2020

@pacien can you add a release note for 20.03 that mentions the breaking change?

@rnhmjoj rnhmjoj added this to the 20.03 milestone Mar 1, 2020
@pacien pacien mentioned this issue Mar 1, 2020
Merged
10 tasks
@pacien
Copy link
Contributor

pacien commented Mar 1, 2020

For the release notes: #81473

pacien added a commit to pacien/nixpkgs that referenced this issue Mar 1, 2020
@pacien
riot-web: mention incompatible config change in release notes
6d4fd13 
Mention the changes introduced in commit c9e5cca.

GitHub: closes NixOS#81416
rnhmjoj pushed a commit that referenced this issue Mar 2, 2020
@pacien @rnhmjoj
riot-web: mention incompatible config change in release notes
61cc1f0 
Mention the changes introduced in commit c9e5cca.

GitHub: closes #81416
(cherry picked from commit 6d4fd13)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
0.kind: bug
Projects
None yet
Milestone
20.03
4 participants
@mguentner @pacien @rnhmjoj @worldofpeace