-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add Reply-To function #199
Conversation
Thanks a lot for the contribution! I was trying to test this out, but I couldn't make it work. |
Actually I was wrong, it does seem to work, it's just undocumented. I was able to create a filter that does just that and it seems to work. FFR, here's the exported version: <feed xmlns='http://www.w3.org/2005/Atom' xmlns:apps='http://schemas.google.com/apps/2006'>
<title>Mail Filters</title>
<id>tag:mail.google.com,2008:filters:...</id>
<updated>2021-09-27T19:35:54Z</updated>
<author>
<name>Me</name>
<email>me@gmail.com</email>
</author>
<entry>
<category term='filter'></category>
<title>Mail Filter</title>
<id>tag:mail.google.com,2008:filter:z...</id>
<updated>2021-09-27T19:35:54Z</updated>
<content></content>
<apps:property name='hasTheWord' value='replyto:mbrt/gmailctl'/>
<apps:property name='shouldNeverSpam' value='true'/>
<apps:property name='sizeOperator' value='s_sl'/>
<apps:property name='sizeUnit' value='s_smb'/>
</entry>
</feed> |
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.
I'm looking through the changes and they look good!
There's only one place I could see missing:
- README.md, section Tests, about the new
replyto
field in the test object.
…ljr/gmailctl into add-reply-to-function
Hi @mbrt was it as simple as updating the list of fields available for the test object? Or is there an actual test object that needs to be updated? |
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.
Tests are also failing. See the results here.
One way to fix this is to do the following:
go test ./... --update
go test ./...
The first command should trigger the regeneration of the golden test files here: https://github.com/mbrt/gmailctl/blob/master/pkg/config/lib_test.go#L18. The second will run all the tests. If nothing fails we should be good to go!
Alright, everything should be fixed now. I was updating all of the same files that were updated when BCC was added as an operator and It seems I updated an autogenerated file used by the Go tests. |
Sorry for the delay in responding. Actually the file you updated was good, it's the input to the test. The result is checked against the JSON files that are updated through the commands in the comment above. If you remove it, there's no test for the new field you added, so a future change might break the behavior. Could you please add it back and fix the tests? Then we are truly good to go. |
Ok I think the tests are fixed now! :) |
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, thanks!
Thanks for the contribution! :) |
Thanks, @peteroneilljr ! |
Adding Reply-To function mentioned in issue #140