matthiasblaesing commented on PR #5609: URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1492853749
@jtulach thank you for considering the suggestions and for the separation of the classloader. That indeed looks better readable. For the modifications to the CI/CD script, I'd like to try to explain what happens there: We have several tests, that are run only if certain labels are set on the corresponding PR. The idea here is, that github and Apache can't provide infitite processing power and we need to use it were it counts. Two big blocks are PHP and full java tests. Both run in the range of an hour and should not be run on all PRs, just PRs with the label `PHP` or `Java`. We had already a split of the CI/CD runs into separate parts to allow parallel execution and @mbien added protection to the execution blocks, that checks whether the label is set: https://github.com/apache/netbeans/blob/e1b594034bdce520dbfbc720a6cd7b7b894fc756/.github/workflows/main.yml#L791-L794 This block says: The `platform-modules-test1` job is only run if the label `Platform` or `ci:all-tests` is set. Additionally this will be run on non-PR events, for example the merge to master (the last part of the if). Yes you see rust in the full CI/CD configuration, as the first steps for rust support were added by @vieiro, but are only build by the `full` cluster configuration, not by the `release` config. The line you referenced declares a variable `test_rust`, nothing more, apart from that it is identical to the guard shown above for `Platform`. I hope the clears the idea a bit. I just saw your comment over in my "test" PR: > Is it wise to run the job only when a special label is added? We wouldn't catch https://github.com/apache/netbeans/pull/5748 so quickly without running this job. I'm in a yes/no situation. I agree, that the nbjavac run was helpful to catch this. On the other hand I can understand @mbien concerns regarding false positive failures from the nbjavac run. A valid middle ground may be, to change this: https://github.com/apache/netbeans/blob/3027e4274aaa0e8a40c1ab700db67b954d76784c/.github/workflows/main.yml#L162 to: ``` if: ${{ contains(github.event.pull_request.labels.*.name, 'nb-javac') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }} ``` This is a compromise for both sides: On the one hand the nbjavac job will only be run on specially labeled PRs (`nb-javac` or `ci:all-tests`). On the other hand it will be run on merges to `master` (at least that is my understanding). So normal PRs will not yield more runtime, yet if the unlikely (it happend twice now) case happens, that an API is used which does not match the declared JDK, it can still be caught. The price is IMHO ok, my notebook needs 9 minutes for the NetBeans build and 3 minutes for the commit-validation run, on server class hardware I assume less. @mbien @neilcsmith-net @jtulach what do you think? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
