Repository navigation
Refactor createReqBody - #1818
Conversation
Coverage Report for CI Build 39146595171Coverage decreased (-0.03%) to 60.381%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
|
Hello David, I noticed the Coveralls report says the coverage has decreased. I checked with previous reports locally, and it seems like the lines were uncovered before. Would you like me to add the tests to cover these lines? |
… into refactor-createReqBody
| fasCalendarUrl: "https://artsci.calendar.utoronto.ca/" | ||
| programsUrl: "https://artsci.calendar.utoronto.ca/listing-program-subject-areas" | ||
|
|
||
| # Data Fetching Parameters |
There was a problem hiding this comment.
use lowercase for "fetching parameters"
| ] | ||
| createReqBody :: Int -> IO Value | ||
| createReqBody page = do | ||
| sessions <- timetableSessionsValue <$> loadConfig |
There was a problem hiding this comment.
In response to your comment in our meeting: you can load the configuration in makeRequest and then pass in the sessions as a plain [T.Text] argument here, so the function remains pure: [T.Text] -> Int -> Value.
There was a problem hiding this comment.
Hello David, I have adjusted the pipeline to export the session value from Config.hs instead of having makeRequest do another config reading. However, I am not sure if this is what you wanted. Would it be better to actually just keep the createReqBody impure or actually read the config in makeRequest?
david-yz-liu
left a comment
There was a problem hiding this comment.
Nice work @PeanutPiglet, yes this approach for keeping createReqBody pure is good 👍
Proposed Changes
Refactored the
createReqBodyunderapp/Config.hsto use settings fromconfig.yamlType of Change
Checklist
Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)