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

added Factorial function #58

Merged
merged 7 commits into from
Dec 17, 2021
Merged

added Factorial function #58

merged 7 commits into from
Dec 17, 2021

Conversation

Friendly-Banana
Copy link
Contributor

Provides one part of #26
The new Factorial function uses the symbol ! and provides basic integer factorials. Non-integers are first rounded and then used as integers.

Copy link
Owner

@fkleon fkleon left a comment

Choose a reason for hiding this comment

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

Looks good pending one comment, thanks for this addition.

Could you also add some unit tests for the parser (into parser_test_set.dart) and the new expression (into expression_test_set.dart)?

lib/src/functions.dart Outdated Show resolved Hide resolved
@Friendly-Banana
Copy link
Contributor Author

Ok, I think I added all tests.

Copy link
Owner

@fkleon fkleon left a comment

Choose a reason for hiding this comment

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

Thanks for adding those tests, this looks good. They've surfaced a few more issues to fix but you're almost there :)

lib/src/functions.dart Show resolved Hide resolved
lib/src/functions.dart Outdated Show resolved Hide resolved
lib/src/functions.dart Outdated Show resolved Hide resolved
@fkleon fkleon merged commit a1498d5 into fkleon:master Dec 17, 2021
@fkleon
Copy link
Owner

fkleon commented Dec 17, 2021

@Just-Learned-It Thanks for your contribution! Travis CI wasn't working with the new dart format command so I've switched to GitHub Actions and fixed the minor formatting issues on master directly.

@Friendly-Banana
Copy link
Contributor Author

Glad I could help. I'm using the librabry myself so it's somewhat selfish :). Thanks for all the corrections.
Regarding the format, I had thought about opening another issue for all the format warnings. But you fixing it directly is even better.

@fkleon
Copy link
Owner

fkleon commented Dec 20, 2021

This is included in version 2.3.0 that I've just released to pub

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