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 deprecations.yaml as a single-source-of-truth #3872

Merged
merged 1 commit into from
May 29, 2024
Merged

Conversation

jathak
Copy link
Member

@jathak jathak commented May 21, 2024

Instead of having the list of deprecations repeated in several different places across all of our repos, the new spec/deprecations.yaml file will be the only place we need to update it, with all other uses generating code based on it.

Instead of having the list of deprecations repeated in several different
places across all of our repos, the new `spec/deprecations.yaml` file
will be the only place we need to update it, with all other uses
generating code based on it.
@ntkme
Copy link
Contributor

ntkme commented May 22, 2024

Should this file also have information about obsoleteIn version in addition to deprecatedIn version?

@jathak
Copy link
Member Author

jathak commented May 22, 2024

The code generators support an obsolete property under dart-sass (see the example in the comment at the top), but there aren't actually any obsolete deprecations yet.

@ntkme
Copy link
Contributor

ntkme commented May 22, 2024

It might be better to define the key with an empty value in yaml. This way we don't have conditional key in schema, and will making paring logic easier and more consistent in other programming language. In some programming language, the yaml object mapper may require the key to exist.

E.g.

call-string:
  description: Passing a string directly to meta.call().
  dart-sass:
    status: active
    deprecated: 0.0.0
    obsolete:

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 prefer omitting missing fields to setting them to the empty string.

@ntkme
Copy link
Contributor

ntkme commented May 22, 2024

Another question I have is that how do we make sure the host would use the same version of deprecations.yaml if the host is not build at the same time as dart-sass? Should this be versioned / tagged like the embedded protocol?

@ntkme
Copy link
Contributor

ntkme commented May 23, 2024

We only have a single primary implementation of sass. Other third party implementations may not necessarily implements the same language features, and thus may not use same list of deprecations. It is a bit awkward if third party implementation has to update sass/sass repo for their own deprecations that may have nothing to do with dart-sass.

It's probably better to removing the "dart-sass" nesting in the yaml, and put this file directly in the dart-sass repo so that the file is tagged together with dart-sass version? For third party implementations, they can provide their own deprecations.yaml in their own repo if they wish.

@jathak
Copy link
Member Author

jathak commented May 23, 2024

I don't think versioning the YAML file is necessary, since an embedded host could always just ignore any Dart Sass versions in deprecated or obsolete older than the compiler version they're using.

The deprecations list is also part of the JS API, so it makes sense for it to live here, rather than in sass/dart-sass; we still want a dart-sass section because those versions aren't specified in the JS API, as some other non-Dart Sass implementation could support the JS API in the future, and it could use the same language-level deprecations but with different versions than Dart Sass.

@nex3
Copy link
Contributor

nex3 commented May 29, 2024

I agree with @jathak. Deprecations generally represent behavioral differences between versions of the specification, and so can be expected to be fairly consistent across implementations other than version numbers. I think having some edge cases where we may need to add implementation-specific deprecations that aren't necessary in other implementations is a fine price to pay for having deprecations in general be a well-defined part of the JS API.

@jathak jathak merged commit 87699d7 into main May 29, 2024
9 checks passed
@jathak jathak deleted the deprecations-yaml branch May 29, 2024 21:23
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.

None yet

3 participants