Skip to content
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

[Update] Send long log as file #41

Merged

Conversation

kiritokatklian
Copy link
Contributor

@kiritokatklian kiritokatklian commented Mar 17, 2023

  • Added new tests
  • Updated RichRecordConverter to send text longer than 2000 as file too

Addresses #20

- Added new tests
- Updated RichRecordConverter to send text longer than 2000 as file too
@vpratfr vpratfr merged commit aaa08fd into marvinlabs:master Apr 18, 2023
@vpratfr
Copy link
Member

vpratfr commented Apr 18, 2023

Thanks a lot. Just released 1.4.1 with that.

@ciceroborges
Copy link

Hey @vpratfr @kiritokatklian, first of all thanks for the update, it looks great.

I just tested it and a doubt came up. I noticed that when the characters are exceeded the log is duplicated, it goes as a common message and then as an attachment, is it supposed to be like this? I think it would be better if it was only the attachment, right?

(Lorem Ipsum 5000 characters) - https://i.imgur.com/wifHsmu.png

What do you think? Should I open an issue?

@vpratfr
Copy link
Member

vpratfr commented Apr 18, 2023

Open an issue, yes.

Maybe in that case the preview could be shorter indeed.

= have a preview cropped to 250 characters + full attachment

I would also rename the attachment (stacktrace.txt is not a stacktrace only anymore)

Feel free to submit a PR too afterwards

@kiritokatklian
Copy link
Contributor Author

@ciceroborges yes, I didn't change the default behavior. Only added the logic to send the same string as a file if it's too long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants