Closed ericfeczko closed 3 years ago
I approved and added some comments. Yes, needs to be tested before merging.
From: ericfeczko @.***> Sent: Wednesday, July 7, 2021 4:24 PM To: DCAN-Labs/dcan_bold_processing Cc: Kathy Snider; Review requested Subject: [EXTERNAL] [DCAN-Labs/dcan_bold_processing] changed regular expression pattern to better capture run numbers (#12)
Regular expression pattern improved to handle multiple numbers, does not require padding, etc. This introduces a new limitation where task names cannot contain numerics -- consider revising with an alternative, or posting as an additional requirement/known issue.
Adding Kathy and Anders as potential reviewers. Amanda, feel free to reassign as needed -- this should be tested before merging.
You can view, comment on, or merge this pull request online at:
https://github.com/DCAN-Labs/dcan_bold_processing/pull/12
[https://opengraph.githubassets.com/06064fa28eb024eb547ad4dddedeae1d4a22e670b1c0843c4fb06010e5a39daa/DCAN-Labs/dcan_bold_processing/pull/12]https://github.com/DCAN-Labs/dcan_bold_processing/pull/12
changed regular expression pattern to better capture run numbers by ericfeczko · Pull Request #12 · DCAN-Labs/dcan_bold_processing · GitHubhttps://github.com/DCAN-Labs/dcan_bold_processing/pull/12 github.com Regular expression pattern improved to handle multiple numbers, does not require padding, etc. This introduces a new limitation where task names cannot contain numerics -- consider revising with an alternative, or posting as an additional requirement/known issue. Adding Kathy and Anders as potential reviewers. Amanda, feel free to reassign as needed -- this should be tested before merging.
Commit Summary
File Changes
Patch Links:
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/DCAN-Labs/dcan_bold_processing/pull/12, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJAQP4AEWLQVJFEUE4UQHD3TWTO3JANCNFSM477TCGGA.
just realized I needed to make a couple of more edits, will cancel this request and reopen shortly
Regular expression pattern improved to handle multiple numbers, does not require padding, etc. This introduces a new limitation where task names cannot contain numerics -- consider revising with an alternative, or posting as an additional requirement/known issue.
Adding Kathy and Anders as potential reviewers. Amanda, feel free to reassign as needed -- this should be tested before merging.