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

chore: fix the build problems #201

Merged
merged 4 commits into from Nov 25, 2021

Conversation

zambrovski
Copy link
Contributor

Hi Folks,

as promised, my first attempt. I believe this is a good step towards a more stable and comfortable build. I added a new issue #200 to clean-up pom (especially duplicate declarations and places of version declarations) and focus in this PR on the build process only.

Things changed:

  • reorganized build to run in two phases (itests run in a separate profile itest)
  • create coverage aggregate report (a separate profile activates creation of an coverage aggregate report -Pcoverage-aggregate)
  • made PRs be not checked by Sonar (since the secret is not available by PR)
  • separate Sonar run into a separate GH step
  • made examples exclude-able from deployment to central (no reason to deploy examples to central)
  • fix Separate execution of unit tests from integration tests #199

I believe if you like this structure, it might be worth to create a Axon-Community Extension Parent POM project - to simplify the setup of the extension and containing explanations of usage and setup. This would ease your handling of extensions, if they are at least having the same parent and you could move some of the POM magic up to it...

@zambrovski
Copy link
Contributor Author

By the way - as usual for the PR with the build system, it might be needed some special attention for the steps used during the deployment/release procedure.... You can simulate these steps with sometimes, but in the end it might be tricky to debug and judge everything in advance.

Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Bunch of minor comments to cover before I'll approve.

CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/workflows/pullrequest.yml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
coverage-report-generator/pom.xml Show resolved Hide resolved
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@zambrovski
Copy link
Contributor Author

Is there anything I can do here to progress further here?

Copy link
Contributor

@lfgcampos lfgcampos left a comment

Choose a reason for hiding this comment

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

LGTM... although I feel like we added a lot to our pom, sounds like this is the best way to fix this and split our tests!

.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/pullrequest.yml Show resolved Hide resolved
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

My concerns have been addressed, hence approving.

coverage-report-generator/pom.xml Show resolved Hide resolved
@smcvb smcvb merged commit 82d42c6 into AxonFramework:master Nov 25, 2021
@smcvb
Copy link
Member

smcvb commented Nov 25, 2021

Thanks for the effort and patience here @zambrovski! Just went ahead and merged the changes after approval.

@zambrovski zambrovski deleted the feature/separate-build-profiles branch December 13, 2021 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate execution of unit tests from integration tests
3 participants