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

Merge the Dart Sass embedded compiler repo into the Dart Sass repo #1955

Merged
merged 176 commits into from
May 15, 2023
Merged

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented May 10, 2023

nex3 and others added 30 commits May 7, 2019 16:53
This implements the packet scheme that the embedded protocol uses when
communicating over stdin and stdout.
Currently this doesn't actually invoke the compiler, it just always
returns the expanded output for "a {b: c}".
Add a very basic protocol dispatcher and a few tests
This now supports real CompileRequest and CompileResponses, although
it's missing features like importers, custom functions, and source
maps.
This doesn't yet support first-class function values.
* Remove support for InboundMessage.FunctionCallRequest

Context: sass/embedded-protocol#28

* Remove support for CanonicalizeResponse.file

Context: sass/embedded-protocol#25

* Stub CompileRequest.Importer.fileImporterId support
Release a beta version
Make better use of cli_pkg
The previous token didn't seem to be recognized. See
https://travis-ci.com/github/sass/dart-sass-embedded/jobs/364028782.

It's unclear why it didn'twork, but I'm hoping regenerating fixes it.
I realized the problem: this repo uses travis-ci.com, not
travis-ci.org, but the Travis CLI defaults to encrypting using .com
credentials. This time I regenerated the token using --pro, which
should work.
@nex3 nex3 requested a review from jathak May 10, 2023 22:26
@nex3 nex3 marked this pull request as draft May 10, 2023 23:25
@nex3 nex3 removed the request for review from jathak May 10, 2023 23:25
@nex3 nex3 force-pushed the embedded branch 9 times, most recently from 3b8f0a2 to 7cd3949 Compare May 11, 2023 00:08
@nex3 nex3 force-pushed the embedded branch 3 times, most recently from ac56a86 to 00fcf59 Compare May 11, 2023 01:34
@nex3 nex3 marked this pull request as ready for review May 11, 2023 01:40
@nex3 nex3 requested a review from jathak May 11, 2023 01:40
@@ -103,6 +152,72 @@ jobs:
run: npm run js-api-spec -- --sassSassRepo ../language --sassPackage ../build/npm
working-directory: sass-spec

# The versions should be kept up-to-date with the latest LTS Node releases.
# They next need to be rotated October 2021. See
Copy link
Member

Choose a reason for hiding this comment

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

Should we rotate these now, or save that for a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separate PR, I think.

pubspec.lock Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why we're now checking this in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, got caught in the merge.

run: |
echo "sass=${GITHUB_REF##*/}" | tee --append $GITHUB_OUTPUT
echo "sass_api=$(curl --fail --silent --show-error --location https://raw.githubusercontent.com/sass/dart-sass/${GITHUB_REF##*/}/pkg/sass_api/pubspec.yaml | yq .version)" | tee --append $GITHUB_OUTPUT
run: echo "::set-output name=version::${GITHUB_REF##*/}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: echo "::set-output name=version::${GITHUB_REF##*/}"
run: echo "version=${GITHUB_REF##*/}" | tee --append $GITHUB_OUTPUT

::set-output is deprecated:

https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we may have a few of these. Maybe we should do this separately in a different PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'd prefer to save this for a separate PR.

@nex3 nex3 merged commit bd771af into main May 15, 2023
@nex3 nex3 deleted the embedded branch May 15, 2023 21:49
@ntkme
Copy link
Contributor

ntkme commented May 15, 2023

@nex3 Are we going to release 1.63.0 as is or are we waiting for some other changes to land? I think we should cut a release now as we already archived dart-sass-embedded repo.

@nex3
Copy link
Contributor Author

nex3 commented May 22, 2023

We're going to release it as soon as we implement the other embedded protocol changes. I want to make sure all the breaking changes are rolled into a single release.

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.

8 participants