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

DRAFT: Refactor to move Classpath Jar reports into the API module #151

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

kittylyst
Copy link
Collaborator

Some of these classes seem to make more sense in the API module, as we need them in the Standard Agent (which is Java 8).

Can people review, please - I feel like this warrants some discussion.

@kittylyst kittylyst requested review from jponge and ehsavoie November 9, 2023 15:53
@jponge
Copy link
Collaborator

jponge commented Nov 13, 2023

Funnily the conventional commit check configuration hasn't spotted that the commit message is not in the expected format, it should be something refactor!: move classpath Jar reports into the API module like according to https://www.conventionalcommits.org/en/v1.0.0/#summary

Doing a proper review later this morning.

Copy link
Collaborator

@jponge jponge left a comment

Choose a reason for hiding this comment

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

Thanks @kittylyst

From a client maintenance perspective I don't see any objection for moving these classes from the runtime to the API module.

  1. This would trigger a 2.0.0 as it's a breaking change and this project shall adhere to semantic versioning.
  2. The commit message must follow conventional commit specs (see my comment above / refactor!: move classpath Jar reports into the API module should be a good start, ! is for a breaking change)
  3. I'll let @ehsavoie give his take on the impact on the EAP 7/8 clients, if any.

Copy link
Collaborator

@jponge jponge left a comment

Choose a reason for hiding this comment

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

Approved but do not merge right now

@jponge jponge force-pushed the classpath_report_to_api branch from 4b71134 to d7d5194 Compare November 15, 2023 12:42
@jponge jponge force-pushed the classpath_report_to_api branch from d7d5194 to f62808e Compare November 15, 2023 12:53
@kittylyst kittylyst merged commit 41e8906 into main Nov 15, 2023
2 checks passed
@kittylyst kittylyst deleted the classpath_report_to_api branch October 7, 2024 14:25
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