You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/06/03 10:15:16 UTC
[GitHub] [maven-gh-actions-shared] cstamas opened a new pull request, #46: Add more FF flags
cstamas opened a new pull request, #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46
Add more flags:
* to control is FF step needed at all
* to control is FF required to build site at all
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add more FF flags [maven-gh-actions-shared]
Posted by "Marcono1234 (via GitHub)" <gi...@apache.org>.
Marcono1234 commented on code in PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#discussion_r1673141637
##########
.github/workflows/maven-verify.yml:
##########
@@ -199,7 +212,8 @@ jobs:
name: ${{ matrix.os }} jdk-${{ matrix.jdk }}-${{ matrix.distribution }}
timeout-minutes: ${{ inputs.timeout-minutes }}
runs-on: ${{ matrix.os }}
-
+ if: always() && ( !inputs.ff-run || needs.fail-fast-build.result == 'success' )
Review Comment:
It seems this `if: always()` here has the unfortunate side effect that it is impossible to cancel the workflow, see https://github.com/orgs/community/discussions/26303. That is quite problematic because once the matrix starts, you have potentially dozens of long running uncancelable jobs.
As suggested in that discussion, maybe `!cancelled()` could be an alternative to `always()` here?
What was the intention for `if: always()`? Note that there is already `strategy.fail-fast` being set below.
Sorry for commenting on this old PR, but since this introduced it originally, I hope that is ok.
Edit: Have created #108 now, proposing to replace the `if: always()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1146338232
I added one expression more - now verify is executed when ff is skipped ...
to test:
- ff - enable and ff - fail
- ff - enabled - PR from external fork
- ff - disabled - PR from external fork
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on a diff in pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#discussion_r888917900
##########
.github/workflows/maven-verify.yml:
##########
@@ -147,7 +159,7 @@ jobs:
runs-on: ${{ inputs.ff-os }}
timeout-minutes: ${{ inputs.ff-timeout-minutes }}
# execute on any push or pull request from forked repo
- if: github.event_name == 'push' || ( github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork )
+ if: ff-run && ( github.event_name == 'push' || ( github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork ) )
Review Comment:
`inputs.ff-run`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1145938583
please change in one project reference from `v2` to your branch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1147659197
I see one more issue, when ff is disabled and we create PR from branch in repo we have build matrix for both events push and pull-request, like in https://github.com/apache/maven-indexer/pull/207/checks
<img width="328" alt="image" src="https://user-images.githubusercontent.com/3578921/172206920-64d8d768-ce9e-4efc-92f2-c3fc9b76f84f.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1145939413
I don't sure if rest of steps will be executed if one will be skipped
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on a diff in pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#discussion_r888918237
##########
.github/workflows/maven-verify.yml:
##########
@@ -174,6 +186,7 @@ jobs:
- name: Build Maven Site
run: ./mvnw --errors --batch-mode --show-version ${{ inputs.maven_args }} ${{ inputs.ff-site-goal }}
+ if: ff-site-run
Review Comment:
`inputs.ff-site-run`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] cstamas commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1146275569
> please change in one project reference from `v2` to your branch
Seems not working :disappointed:
https://github.com/apache/maven-indexer/pull/207/commits/f2a8f3877eaf07a5414c758522210946b297b87a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] cstamas commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1154205558
But isn't this true even now, without this PR? As I see this is an OR condition and that area is not changed, so as I read, even now it should run steps for both, "push" and "pull_request" events...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add more FF flags [maven-gh-actions-shared]
Posted by "Marcono1234 (via GitHub)" <gi...@apache.org>.
Marcono1234 commented on code in PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#discussion_r1673141637
##########
.github/workflows/maven-verify.yml:
##########
@@ -199,7 +212,8 @@ jobs:
name: ${{ matrix.os }} jdk-${{ matrix.jdk }}-${{ matrix.distribution }}
timeout-minutes: ${{ inputs.timeout-minutes }}
runs-on: ${{ matrix.os }}
-
+ if: always() && ( !inputs.ff-run || needs.fail-fast-build.result == 'success' )
Review Comment:
It seems this `if: always()` here has the unfortunate side effect that it is impossible to cancel the workflow, see https://github.com/orgs/community/discussions/26303. That is quite problematic because once the matrix starts, you have potentially dozens of long running uncancelable jobs.
As suggested in that discussion, maybe `!cancelled()` could be an alternative to `always()` here?
What was the intention for `if: always()`? Note that there is already `strategy.fail-fast` being set below.
Sorry for commenting on this old PR, but since this introduced it originally, I hope that is ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] slawekjaranowski commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1159075685
build on pull_request event is skiped
https://github.com/apache/maven-dependency-plugin/pull/221/checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] cstamas merged pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-gh-actions-shared] cstamas commented on pull request #46: Add more FF flags
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #46:
URL: https://github.com/apache/maven-gh-actions-shared/pull/46#issuecomment-1146675834
For indexer case this works now! Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org