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.

Sign up for GitHub

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

Jump to bottom

Make text in event non mandatory #160

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Make text in event non mandatory #160

merged 1 commit into from
Oct 18, 2021

Conversation

hush-hush
Copy link
Member

No description provided.

@hush-hush hush-hush force-pushed the maxime/remove-mandatory-event-text branch 4 times, most recently from 5179213 to 46adb65 Compare September 23, 2021 13:16
@truthbk
Copy link
Member

truthbk commented Sep 26, 2021

There's a test you might want to fix:

Results :

Failed tests: 
  EventTest.fails_without_text Expected exception: java.lang.IllegalStateException

Tests run: 138, Failures: 1, Errors: 0, Skipped: 40

@hush-hush could you please elaborate a little bit on the use-case? Is this for consistency with other clients?

@hush-hush
Copy link
Member Author

@hush-hush could you please elaborate a little bit on the use-case? Is this for consistency with other clients?

This was requested by product and is already implemented in all other clients.

@hush-hush hush-hush force-pushed the maxime/remove-mandatory-event-text branch 10 times, most recently from 1df2232 to 5b8d8f1 Compare October 15, 2021 10:16
truthbk
truthbk reviewed Oct 15, 2021
View reviewed changes
Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're almost there! Just a small clarification, and hopefully a missing test we should add.

src/test/java/com/timgroup/statsd/EventTest.java Show resolved Hide resolved
@hush-hush hush-hush force-pushed the maxime/remove-mandatory-event-text branch 2 times, most recently from 2478cc9 to 3fa3e16 Compare October 18, 2021 10:32
@hush-hush hush-hush force-pushed the maxime/remove-mandatory-event-text branch from 3fa3e16 to ad866b7 Compare October 18, 2021 11:01
@truthbk truthbk merged commit 7265535 into master Oct 18, 2021
@truthbk truthbk deleted the maxime/remove-mandatory-event-text branch October 18, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vickenty vickenty vickenty approved these changes

@truthbk truthbk truthbk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hush-hush @truthbk @vickenty