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

Fix naming of tracking function argument in reaction #398

Merged
merged 3 commits into from
Jan 16, 2020

Conversation

MaikuB
Copy link
Collaborator

@MaikuB MaikuB commented Jan 16, 2020

Noticed that the argument in reaction was named as predicate when it doesn't return a boolean value. Changed this to fn though we may want to change this something else. Alternatives I can think of are trackingFunction/trackingFn or dataFunction/dataFn

Sidenote: suggest agreeing on a standard in the API docs. I noticed there are cases where functions are referred to as <function> and other places use <function>(). I would suggest the former and it's also the approach used in the Flutter docs

@codecov
Copy link

codecov bot commented Jan 16, 2020

Codecov Report

Merging #398 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #398   +/-   ##
=======================================
  Coverage   97.87%   97.87%           
=======================================
  Files          46       46           
  Lines        1644     1644           
=======================================
  Hits         1609     1609           
  Misses         35       35
Flag Coverage Δ
#flutter_mobx 100% <ø> (ø) ⬆️
#mobx 97.77% <100%> (ø) ⬆️
#mobx_codegen 100% <ø> (ø) ⬆️
Impacted Files Coverage Δ
mobx/lib/src/api/reaction.dart 100% <100%> (ø) ⬆️
mobx/lib/src/core/reaction_helper.dart 96.36% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a08a9a5...c8a68ee. Read the comment docs.

@pavanpodila pavanpodila merged commit 6c90b66 into master Jan 16, 2020
@pavanpodila pavanpodila deleted the reaction_tracking_function branch January 16, 2020 15:45
@pavanpodila
Copy link
Member

Sidenote: suggest agreeing on a standard in the API docs. I noticed there are cases where functions are referred to as <function> and other places use <function>(). I would suggest the former and it's also the approach used in the Flutter docs

Agree with the consistency of style. We can do it the way Flutter docs do it. Sounds like a new PR :-)

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

2 participants