Sundry config fixes #776
Sundry config fixes #776
Conversation
There was a problem hiding this comment.
a couple suggestions for simplifying the logic in the new macro slightly, and i'd lean slightly towards testing the small part of the code that seems responsible for the coverage drop. but otherwise, looks fine to me, thanks for all the iterating and updating!
also @jacobthill i'm leaving this as a comment -- i went to request changes, but this is technically my PR, even though the work and the commits are all yours, since i did the branch wrangling (and so i'm not allowed to approve or request changes).
There was a problem hiding this comment.
🎉 nice, looks good! thanks for the touchups, @jacobthill!
one thing: looks like a little bit of detritus got committed, maybe errant copy/paste on the CLI? see image...
jmartin-sul
force-pushed
the
sundry-config-fixes
branch
from
0e53198 to
7558d30
Compare
Why was this change made?
this splits out the traject config and macro changes @jacobthill made on top of the first two commits of #762. since i didn't have any hand in those follow-on changes, and since they are in separate parts of the code changes i made in my first two commits, i can review them separately, and hopefully get this PR merged on its own.
How was this change tested?
CI, running transform locally on laptop
Which documentation and/or configurations were updated?
lots of traject configs