-
Notifications
You must be signed in to change notification settings - Fork 7
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
Future actions #167
Future actions #167
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #167 +/- ##
==========================================
+ Coverage 55.25% 57.60% +2.35%
==========================================
Files 14 14
Lines 628 710 +82
==========================================
+ Hits 347 409 +62
- Misses 237 245 +8
- Partials 44 56 +12
☔ View full report in Codecov by Sentry. |
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.
Looks good so far
filedrive: wrap ListDir to exclude future dated files
Co-authored-by: Adam Shannon <[email protected]>
Co-authored-by: Adam Shannon <[email protected]>
pkg/response/match/matcher.go
Outdated
/* | ||
* Matching will find at most 2 Actions: 1 Copy Action and 1 Return/Correction Action. | ||
* If the Return/Correction Action as no Delay, the Copy Action will be excluded. | ||
* | ||
* Valid combinations include: | ||
* 1. Copy | ||
* 2. Return/Correction w/ Delay | ||
* 3. Return/Correction w/o Delay | ||
* 4. Copy and Return/Correction w/ Delay | ||
* 5. Nothing | ||
* | ||
* Invalid combinations are: | ||
* 1. Copy + Return/Correction w/o Delay | ||
* 2. Copy w/ Delay (validated when reading configuration) | ||
*/ |
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.
Did we want to link to the readme in a shorter version of this comment? We need to keep the two in sync since they're so similar.
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.
That's not a bad idea. Is there any particular trick to linking in go (like using HTML or a specific tag), or is just "See README.md for the matching rules"?
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.
That works, but Github headers are link themselves.
https://github.com/moov-io/ach-test-harness#project-status
Tested with a live Docker container and FTP client and saw the expected behavior on the "delayed" files |
func (a Action) Context() map[string]log.Valuer { | ||
logFields := log.Fields{} | ||
|
||
// Safely retrieve several values that are needed for the debug log below | ||
if a.Delay != nil { | ||
var delayTime = a.Delay.String() | ||
logFields["delay"] = log.String(delayTime) | ||
} |
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.
Usually changes like this should be in a separate PR since they're not related to adding delay.
Changes
Support future-dated actions in ach-test-harness (for use in Staging)
Why Are Changes Being Made