-
Notifications
You must be signed in to change notification settings - Fork 13
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
FYST-924 Implement Form 502CR #5010
base: main
Are you sure you want to change the base?
Conversation
Heroku app: https://gyr-review-app-5010-51786d861c24.herokuapp.com/ |
app/lib/efile/md/md502_calculator.rb
Outdated
@@ -74,13 +74,13 @@ def calculate | |||
set_line(:MD502_LINE_22, :calculate_line_22) | |||
set_line(:MD502_LINE_22B, :calculate_line_22b) | |||
|
|||
@md502cr.calculate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 why did you move the calculator here as opposed to after MD502_LINE_21
? do future calculations for 502cr depend on 22 and 22b?
[dust] it could be helpful to add a comment which lines 502CR depends on from 502 and vice versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was more that i was moving it right before L24 because its the first line where md502 needs md502cr calculations but I'll move to being after L21 because L22-23 arent necessarily used for md502cr anyways. thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, that makes sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
everything looks good! i just have one minor comment. thanks @tahsinaislam 😸
Link to pivotal/JIRA issue
https://codeforamerica.atlassian.net/browse/FYST-924
Is PM acceptance required? (delete one)
What was done?
How to test?
Screenshots (for visual changes)