Update github actions configuration #703
Conversation
- update actions steps's tools - add entry for building project with jdk 21 - add the ability to build project on pull request stage fix build issue on matrix
| @@ -1,8 +1,29 @@ | |||
| name: Jakarta Contexts and Dependency Injection CI | |||
|
|
|||
| on: | |||
| push: | |||
There was a problem hiding this comment.
I think the idea here is that all changes go in via PRs so this will only really trigger during a release (since the bot does direct push) and that's kind of pointless. At least from my perspective, your mileage may vary :)
There was a problem hiding this comment.
I activated triggering the ci on a push on a custom branch for a given push of one or many commits, thing that changes from the previous configuration is that now I can see the build status on my own branch before merging it, this will someway prevent me to make a pull request with changes that can break the build or the tests
| @@ -1,8 +1,29 @@ | |||
| name: Jakarta Contexts and Dependency Injection CI | |||
|
|
|||
| on: | |||
| push: | |||
| branches-ignore: | |||
There was a problem hiding this comment.
Why keep below listed combinations in place?
That combination doesn't even match anything in this project, or am I missing something?
There was a problem hiding this comment.
The idea is to trigger the build on a pull request, to be sure the build is going well, and no test is failed. And for some branches we can skip running the ci, if we make a wip/ branche the build will not be triggered, this is a way is to prevent builds on all branches and to save cpu and memory for Github Actions
| pull_request: | ||
| branches: [ master ] | ||
| types: [opened, synchronize, reopened, ready_for_review] |
There was a problem hiding this comment.
I don't think we want CI to trigger for any PR - main branch would be sufficient.
In case we want to push to older version, that will have to have its own CI setup, mainly due to different JDK versions it is supposed to support.
There was a problem hiding this comment.
The main purpose of triggering the CI on a push or pull request is to be sure that every thing is building well before making a pull request, means the building matrix is fine and the tests succeeds, this will give time to contributor to fix things before making a pull request and have an idea about the ci execution result even on a custom branch.
For the merge request on main branch, the CI always needs approval from maintainers
|
I've sent an alternative PR for this - #706 |
|
Agree. I'll close this PR, but thanks a lot for submitting it! |
|
@starksm64 @manovotn @Ladicek, I think it's better to give a change to a new contributor to fix his pull request to fit review remarks rather pick just good part of it and merge it, then drop the original pull request Anyway this is my point of view |
|
Well, I would advocate for the changes I sent and I wanted to have |
This pull request aims to update Github actions configuration, to give the possibility to contributors to check if their contribution on project builds the ci/cd with success, and add step for building using JDK 21 version