-
Notifications
You must be signed in to change notification settings - Fork 115
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
Externalizing dev doc publishing and avoiding re-run CI on merging. #1698
Conversation
Please add one of the following labels to add this contribution to the Release Notes 👇 |
Codecov Report
@@ Coverage Diff @@
## main #1698 +/- ##
==========================================
- Coverage 85.26% 81.17% -4.10%
==========================================
Files 45 45
Lines 7506 7506
==========================================
- Hits 6400 6093 -307
- Misses 1106 1413 +307 |
It doesn't work... Possible solution: |
runs-on: ubuntu-latest | ||
needs: [smoke-tests] | ||
timeout-minutes: 35 |
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.
I understand you do this because you just want all of the different stages to run at the same time... but it is tipically a good practice to make dependent stages. That will reduce runner times consumption (even if your repo is public, do it for environmental reasons 😄 )
If the smoke tests fail.. the build tests are obviously going to fail. Just give it a thought do I understand your change.
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.
I see your point, but waiting 3-4 minutes to start the "real" testing is annoying. If we care about runner consumption, I would break the smoke test matrix in 4+4 or 2+2+2+2 .. but it seems not gain enough.
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.
Sure, yeah, I get it. No problem. It was just a suggestion 😄
name: Dev docs publisher | ||
|
||
on: | ||
workflow_dispatch: | ||
push: | ||
tags: | ||
- "*" | ||
branches: | ||
- main | ||
|
||
env: | ||
DOCUMENTATION_CNAME: 'mapdl.docs.pyansys.com' | ||
|
||
jobs: | ||
upload_dev_docs: | ||
name: "Upload dev documentation" | ||
if: github.ref == 'refs/heads/main' | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Deploy the latest documentation | ||
uses: pyansys/actions/doc-deploy-dev@v2 | ||
with: | ||
cname: ${{ env.DOCUMENTATION_CNAME }} | ||
token: ${{ secrets.GITHUB_TOKEN }} |
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.
This action does not work as you expect... you need to have a "documentation-html" artifact for this to work. This means that you need to build the docs in the same workflow.
If you have doubts let me know.
As it is, this workflow will try to upload a non-existing artifact =)
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.
that is why I did later #1701
now I wonder what is better, if rebuild the docs or just reuse the artifacts from the other workflow.
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.
Ha yeah... Didn't see that.
Your choice I guess. Reusing artifacts from other workflows is "weird" for me - but acceptable if that's what you want 😄
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.
I'm see if I can... and then decide xD
I think rerun the CI workflow on merge (push-branches-main) is a waste of time and resources. So I'm am stopping that.
This requires to externalize the dev-docs publishing because there are done on merging.
Also I now run all the unit tests from the very beginning, and I fixed some typo regarding cache.
Feedback is appreciated @akaszynski @RobPasMue