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

add --random-seed option #2359

Closed
wants to merge 2 commits into from
Closed

add --random-seed option #2359

wants to merge 2 commits into from

Conversation

Joshix-1
Copy link

@Joshix-1 Joshix-1 commented Sep 20, 2024

This is an important change to make the output of dart-sass reproducible.

Copy link
Contributor

@nex3 nex3 left a comment

Choose a reason for hiding this comment

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

I think this is a good concept, but it doesn't take into account running the compiler from the Dart or JS APIs or through the embedded protocol. This will require a proper proposal.

lib/src/executable/compile_stylesheet.dart Show resolved Hide resolved
@Joshix-1
Copy link
Author

Joshix-1 commented Sep 20, 2024

This will require a proper proposal.

Why? I don't see this as a new sass feature, as it doesn't really introduce anything new. The "random" algorithm seems to be an implementation detail. This only adds a flag to configure the random algorithm a bit.

@nex3
Copy link
Contributor

nex3 commented Sep 20, 2024

Anything that's configurable via the CLI also needs to be configurable through the API, and any changes to the JS API need a language proposal because it's part of the Sass specification.

@nex3
Copy link
Contributor

nex3 commented Oct 14, 2024

Since it's been three weeks, I'm going to close this out. We may end up resurrecting it if/when we go through the full proposal process for adding a new configuration option.

@nex3 nex3 closed this Oct 14, 2024
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