Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

Conversation

@maciejwalkowiak
Copy link
Contributor

No description provided.

@maciejwalkowiak maciejwalkowiak changed the title Fix/envelope transport log4j2 tests Fix/envelope transport tests Sep 8, 2020
/**
* Verifies is [SentryEnvelope] contains first event matching a predicate.
*/
inline fun checkEvent(noinline predicate: (SentryEvent) -> Unit): SentryEnvelope {
Copy link
Contributor

Choose a reason for hiding this comment

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

just out of curiosity, could this not live in the sentry-core/test package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can it be then included in other modules? It wouldn't be straightforward with Maven. Perhaps there is a reasonable way to do it with Gradle?

Copy link
Contributor

Choose a reason for hiding this comment

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

mm good catch, I thought testImplementation would be enough, but I think I've seen ways of doing that, just don't remember by heart now.

@marandaneto
Copy link
Contributor

you can ignore LGTM btw, I just deactivated it, their Android setup is broken and I've raised an issue for them

@maciejwalkowiak
Copy link
Contributor Author

@marandaneto could you please approve or tell what needs to be changed? Creating separate module has been suggested by @bruno-garcia.

Having feat/sentry-java broken is currently a blocker for me.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@maciejwalkowiak maciejwalkowiak merged commit c891d6a into feat/sentry-java Sep 9, 2020
@maciejwalkowiak maciejwalkowiak deleted the fix/envelope-transport-log4j2-tests branch September 9, 2020 11:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants