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

Calc functions implementation #1970

Merged
merged 14 commits into from
Aug 9, 2023
Merged

Calc functions implementation #1970

merged 14 commits into from
Aug 9, 2023

Conversation

pamelalozano16
Copy link
Contributor

@pamelalozano16 pamelalozano16 commented May 19, 2023

Context: Calc functions proposal
Tests: sass/sass-spec#1912

  • Added util functions

lib/sass.dart Outdated Show resolved Hide resolved
Copy link
Member

@Goodwine Goodwine left a comment

Choose a reason for hiding this comment

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

In your PR description, add a link to the corresponding sass-spec PR. That way the CI pipeline will run the tests against your other PR rather than HEAD

E.g. add:

Tests: https://github.com/sass/sass-spec/pull/1912

The format is not important IIRC, as long as you have a link to the PR it should work.

lib/sass.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/visitor/async_evaluate.dart Outdated Show resolved Hide resolved
lib/src/visitor/async_evaluate.dart Outdated Show resolved Hide resolved
lib/src/visitor/async_evaluate.dart Outdated Show resolved Hide resolved
Copy link
Member

@Goodwine Goodwine 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 the CI errors are complaining because you must generate the "synchronized" file. The async_evaluate.dart file has a non-async counterpart (eveluate.dart) that is autogenerated.

You can run dart run grinder synchronize to generate it. Then you can add it to this PR

lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/visitor/async_evaluate.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
@pamelalozano16 pamelalozano16 force-pushed the pamelalozano branch 2 times, most recently from f86fc49 to a0eace6 Compare May 19, 2023 21:24
@pamelalozano16 pamelalozano16 changed the title Sqrt calc function implementation Calc functions implementation May 19, 2023
@pamelalozano16 pamelalozano16 force-pushed the pamelalozano branch 5 times, most recently from 4373794 to 885dfc7 Compare May 24, 2023 18:54
lib/src/ast/sass/expression/calculation.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/visitor/async_evaluate.dart Outdated Show resolved Hide resolved
lib/src/value/calculation.dart Outdated Show resolved Hide resolved
lib/src/util/number.dart Outdated Show resolved Hide resolved
lib/src/util/number.dart Outdated Show resolved Hide resolved
Copy link
Member

@jathak jathak left a comment

Choose a reason for hiding this comment

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

Good work so far, round involves a supporting a tricky set of different behaviors, so my most substantial comments are on it.

@pamelalozano16 pamelalozano16 force-pushed the pamelalozano branch 6 times, most recently from acfdde9 to b9b9602 Compare May 26, 2023 17:25
@pamelalozano16 pamelalozano16 force-pushed the pamelalozano branch 2 times, most recently from 12fbaa4 to 6f963c4 Compare July 31, 2023 18:33
@pamelalozano16 pamelalozano16 force-pushed the pamelalozano branch 2 times, most recently from 5048f83 to 9f3341e Compare August 7, 2023 23:51
CHANGELOG.md Outdated Show resolved Hide resolved
pubspec.yaml Outdated
@@ -1,5 +1,5 @@
name: sass
version: 1.64.3
version: 1.65.0-dev
Copy link
Member

Choose a reason for hiding this comment

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

We can release this now, please change the version to 1.65.0 without -dev. And then, modify the following 2 files:

  • pkg/sass_api/pubspec.yaml
    • Update the Sass version to sass: 1.65.0
    • Update the package version to version: 8.1.0
  • pkg/sass_api/CHANGELOG.md
    • Add a new entry for version 8.1.0 saying "No user-visible changes.", you can see other examples of this if you scroll down in the same file.

@pamelalozano16 pamelalozano16 merged commit e4c8cd6 into main Aug 9, 2023
45 checks passed
@pamelalozano16 pamelalozano16 deleted the pamelalozano branch August 9, 2023 21:14
@connorskees connorskees mentioned this pull request Aug 10, 2023
30 tasks
nex3 added a commit that referenced this pull request Aug 17, 2023
nex3 added a commit that referenced this pull request Aug 31, 2023
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

4 participants