-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Only run the install task on the code-quality-reports codebase #4558
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
Only run the install task on the code-quality-reports codebase #4558
Conversation
…r than the whole pom.client.xml, including the code-quality-reports codebase by way of the -Dinclude-non-shipping-modules flag.
mitchdenny
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me. I can replicate this in the analyze step I'm using (eng/pipelines/templates/jobs/archetype-sdk-client.yml.
eng/pipelines/client.yml
Outdated
| # We `install` separately from running `site:site site:stage` so that the `install` brings in the non-shipping-modules, | ||
| # but we don't include them in the Maven site commands (so that we don't generate reports for the non-shipping modules). | ||
| # We `install` the code quality reports tooling into our local m2 cache separately from building the Maven project | ||
| # reports. This means it is available as part of that, but also so that this is not documented in the proejct report. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: proejct -> project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You and your words!
|
whomp whomp. Some build artifacts aren't generated like this. I'll tweak and update PR. |
If this PR builds successfully, I'll do the replication before I merge. |
…at all the maven artifacts are installed.
… testing build works with a reduced set of tasks. This won't be merged as we still need a way to generate the whole site output - but hopefully that can be done nightly instead of on every analyse task.
…y disabled some later steps in the Analyze job to understand performance of this approach
…stalling the client libraries (so that they are not run twice in the Analyze step)
|
@mitchdenny @weshaggard As a proof of concept, this PR takes the Analyze job from around 10:52 to 5:24, so roughly reduced by half. The way I mainly achieved this was by not producing HTML artifacts (e.g. Maven Site, JavaDoc, Jacoco, spotbugs reports or checkstyle reports). What I would like to see is these generated daily, rather than per-PR. Is this something that we could do (in which case, much of what I have commented out in this PR would move into that job instead) |
1 similar comment
|
@mitchdenny @weshaggard As a proof of concept, this PR takes the Analyze job from around 10:52 to 5:24, so roughly reduced by half. The way I mainly achieved this was by not producing HTML artifacts (e.g. Maven Site, JavaDoc, Jacoco, spotbugs reports or checkstyle reports). What I would like to see is these generated daily, rather than per-PR. Is this something that we could do (in which case, much of what I have commented out in this PR would move into that job instead) |
|
The |
|
We should cut client.yml to use the archetype template anyway. I'll do that soon! But these changes look good. |
weshaggard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
|
Build failed due to networking failure. |
|
/azp run java - core - ci |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
@weshaggard What's the best way here to get this merged? Due to the network failure I either need to know the right command to re-run the build, or have it force-merged? |
|
@JonathanGiles You should be able to re-run any failed leg via the Checks tab on github. Assuming that doesn't work then the simply copy the job name listed in the details and call |
Yeah - the 're-run' links don't seem to work for me. I'll re-run it as outlined now. |
|
/azp run azure-sdk-for-java - client |
|
Azure Pipelines successfully started running 1 pipeline(s). |
I've found that the re-run links don't actually work until all the jobs in the particular pipeline has finished. If any of them are still running then it doesn't work. The DevOps team should really work on improving that entire experience though because it doesn't give people any feedback. |
No description provided.