Closed lekoenig closed 2 years ago
Hey Lauren, what kind of turn around were you hoping for on this PR? I would kind of like to take a super deep dive into this repo.
what kind of turn around were you hoping for on this PR? I would kind of like to take a super deep dive into this repo.
That would be awesome if you could do that, Julie! I don't have a specific turn around in mind for this PR. Does 1 week sound like a reasonable time frame? Just let me know if you need more time though. In the meantime, I plan to start drafting a blog post around this phase of the pipeline. I probably won't get to that until late this week or early next week.
That's perfect for me. I can give this a good chunk of time early next week.
Thanks again for these suggestions, @padilla410! I think I've addressed all of your comments and requested changes, including making the function documentation internally consistent and adding text to the top-level README that describes how to customize the following parts of the pipeline:
I opted to simplify the pipeline a bit by moving last_forced_build
from the main pipeline code to an example in the README documentation. We had discussed and approved this option in #62, so this is just an FYI for @lindsayplatt.
OK! I can't say enough good things about this PR. Well done!
This PR cleans up comments and documentation and serves to wrap up the first phase of the pipeline that is focused on the data pulls. I tried to resist the urge to fill the README with everything I might include in a blog post, but I've included additional notes in the README that describe some of the decision making that went into building this pipeline.
Any feedback is welcome for making sure that all targets and functions are sufficiently documented and their usage clear!
In particular, I'm considering moving the implementation and documentation around
last_forced_build
(in_targets.R
) to the README as a way to customize the pipeline without adding unnecessarily complexity to our implementation. Lindsay and I had some back and forth on this in #62, but let me know if you have any thoughts on that based on your review here. Thanks in advance!Closes #22