-
Notifications
You must be signed in to change notification settings - Fork 21
chore: Enable JavaDoc Aggregate with Delombok #969
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
Conversation
CharlesDuboisSAP
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.
Why did you create a standalone job instead of putting it into the release like it was before?
It allows for custom branch/tag selection, but primarily because I felt like I don't want to add another point of failure to the release job again. :/ It's a bit awkward we have so many secondary actions in the release, that are technically not functional requirements. But they break the process if something goes wrong. I'm okay with putting it to the release process, but maybe we can find a way to address this concern in the future. |
|
I like the outlook of having a slimmer release process. It makes both troubleshooting and rerunning the workflow(s) faster. The downside is that it is slightly less automation then, i.e., one more button press the releasing person has to do themself. I would still be of the opinion that having it as a separate workflow is better. If we want to have an aggregated javadoc created for each release, we should then add an additional point to the release PR message in the prepare-release workflow. |
Jonas-Isr
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.
Minor points, besides that I like it :)
| - name: "Create JavaDoc PR" | ||
| id: create-javadoc-pr | ||
| if: ${{ steps.replace-javadoc.outputs.CREATE_PR == 'true' }} | ||
| working-directory: ./.cloud-sdk-docs | ||
| run: | | ||
| PR_TITLE="Java: Update JavaDocs for release ${{ needs.bump-version.outputs.release-version }}" | ||
| PR_BODY="Replace the contents of v${{ steps.determine-major-version.outputs.MAJOR_VERSION }} API docs with the latest release of the SDK." | ||
| PR_URL=$(gh pr create --title "$PR_TITLE" --body "$PR_BODY" --repo "${{ env.DOCS_REPO }}") | ||
| echo "PR_URL=$PR_URL" >> $GITHUB_OUTPUT | ||
| echo "PR: $PR_URL" >> $GITHUB_STEP_SUMMARY | ||
| env: | ||
| GH_TOKEN: ${{ secrets.BOT_SDK_JS_FOR_DOCS_REPO_PR }} |
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.
We should add a TODO to the PR created in the prepare-release workflow to review, approve, and merge this JavaDoc PR after the release.
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.
Since the workflow is triggered async, I can't put a direct URL to the upcoming JavaDoc PR.
However I used a generic link which should suffice, IMHO
… javadoc-aggregate-delombok
Jonas-Isr
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.
LGTM
Context
Follow up from
#866
Backlog:
https://github.com/SAP/cloud-sdk-java-backlog/issues/482
Workflow:
https://github.com/SAP/cloud-sdk-java/actions/workflows/javadoc.yaml
Generated PR in Documentation Portal:
SAP/cloud-sdk#2274
Feature scope:
3.12.xhttps://github.com/SAP/cloud-sdk-java-backlog/issues/490Usage
In upcoming dedicated workflow:
Find HTML in
./target/reports/apidocs/Sample output
apidocs.zip
See Screenshot
Definition of Done
Error handling created / updated & covered by the tests aboveDocumentation updatedRelease notes updated