Skip to content

Biome use schema output#483

Open
Eveeifyeve wants to merge 2 commits intonumtide:mainfrom
DigitalBrewStudios:biome-use-schema-output
Open

Biome use schema output#483
Eveeifyeve wants to merge 2 commits intonumtide:mainfrom
DigitalBrewStudios:biome-use-schema-output

Conversation

@Eveeifyeve
Copy link
Copy Markdown
Contributor

@Eveeifyeve Eveeifyeve commented Mar 4, 2026

This pr uses the schema output that was introduced in: NixOS/nixpkgs#476453 instead of fetchurl.
I also updated nixpkgs in flake.lock due to it being outdated and not working if it's not there.

The development of this feature was supported by DigitalBrewStudios.

Copy link
Copy Markdown
Contributor

@jfly jfly left a comment

Choose a reason for hiding this comment

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

Very nice, thanks. I just don't know what our policy is around how far backwards compatible we strive to be with nixpkgs.

Has NixOS/nixpkgs#476453 been backported to nixos 25.11?

Comment thread programs/biome.nix Outdated
@@ -119,17 +106,11 @@ in
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any value in keeping this schema option around now?

Copy link
Copy Markdown
Contributor Author

@Eveeifyeve Eveeifyeve Mar 4, 2026

Choose a reason for hiding this comment

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

I don't even know what the actual reason for this option, existing. I will give this about 24 hours for someone to respond a actual reason about this option should exist, before I go ahead and remove this in this pr.

@Eveeifyeve
Copy link
Copy Markdown
Contributor Author

Very nice, thanks. I just don't know what our policy is around how far backwards compatible we strive to be with nixpkgs.

Has NixOS/nixpkgs#476453 been backported to nixos 25.11?

It hasn't, but i don't mind if you go ahead and backport it.

@jfly
Copy link
Copy Markdown
Contributor

jfly commented Mar 4, 2026

It hasn't, but i don't mind if you go ahead and backport it.

@brianmcgee, if @Eveeifyeve's PR adding the schema output were backported to nixos 25.11, would this PR be acceptable for merging? Or would we rather wait longer than that?

@Eveeifyeve
Copy link
Copy Markdown
Contributor Author

Eveeifyeve commented Mar 5, 2026

It hasn't, but i don't mind if you go ahead and backport it.

@brianmcgee, if @Eveeifyeve's PR adding the schema output were backported to nixos 25.11, would this PR be acceptable for merging? Or would we rather wait longer than that?

I am just going to backport this anyways, it doesn't hurt to NixOS/nixpkgs#496781

@Eveeifyeve Eveeifyeve force-pushed the biome-use-schema-output branch from 5fac1d4 to 972e814 Compare April 25, 2026 19:23
Comment thread programs/biome.nix Outdated
schema = cfg.validate.schema;
schemaPath = cfg.validate.schema.url or (toString cfg.validate.schema);
schema = "${cfg.package}/share/schema.json";
schemaPath = toString schema;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand the reason for having both schema and schemaPath environment variables. Let's pick one and use it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would say use schema and then toString can be used directly inside.

@Eveeifyeve Eveeifyeve force-pushed the biome-use-schema-output branch from 972e814 to fcb5df1 Compare May 2, 2026 20:17
Copy link
Copy Markdown
Contributor

@jfly jfly left a comment

Choose a reason for hiding this comment

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

I haven't tested this myself, but the changes look good to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants