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

#359 - Modified aissemble-alerting-slack tests to run #418

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

jacksondelametter
Copy link
Contributor

PR is to verify the alerting modules are able to run with JDK 17. Fixed an issue with the extensions-alerting-slack module where the unit tests were not being ran on a build. Cleaned up unused dependencies and added transient dependency declarations in the pom files for the foundation-alerting, extensions-alerting, extensions-alerting-teams, and extensions-alerting-slack modules

</dependency>
<dependency>
<groupId>org.aeonbits.owner</groupId>
<artifactId>owner</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Q/A: Was this added because we reference Owner classes via our usage of Krausening? If so, I think it probably makes more sense to just add Krausening and not Owner, as we'd probably not use Owner outside of Krausening.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is and Krausening is already added as a dependency. I have removed the reference to the Owner classes.

<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>${version.junit}</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

S:

Suggested change
<version>${version.junit}</version>
<version>${version.junit}</version>
<scope>test</scope>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@jacksondelametter jacksondelametter force-pushed the 359-alerting-jdk17-upgrade branch from ed529de to a59e86d Compare October 17, 2024 19:56
@jacksondelametter jacksondelametter force-pushed the 359-alerting-jdk17-upgrade branch from 248cf0a to 52353b7 Compare October 17, 2024 20:29
@jacksondelametter jacksondelametter force-pushed the 359-alerting-jdk17-upgrade branch from 52353b7 to ab48ac6 Compare October 17, 2024 20:30
@jacksondelametter jacksondelametter merged commit e3d113b into dev Oct 17, 2024
@jacksondelametter jacksondelametter deleted the 359-alerting-jdk17-upgrade branch October 17, 2024 20:31
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.

2 participants