-
-
Notifications
You must be signed in to change notification settings - Fork 40
Revamp infra #228
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
Revamp infra #228
Conversation
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.
Some suggestions.
Other than that, LGTM
- name: Prepare notification (only on error) | ||
if: steps.build.outcome == 'failure' | ||
if: always() && steps.build.outcome == 'failure' |
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 always
is really necessary? With the &&
, doesn't ends up going back to being just steps.build.outcome == 'failure'
?
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.
Weird enough, it is necessary as it wouldn't run otherwise.
id: prepare | ||
run: .github/scripts/prepmsg.sh logs/build/err*.txt logs/notify.txt | ||
run: | | ||
scripts/prepmsg.sh logs/sphinxwarnings.txt logs/notify.txt |
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.
Maybe we could/should move this logs/sphinxwarnings.txt
and logs/notify.txt
to environment variables/constants.
Since it's being used on multiple places and different files. That way if we change it, already updates everywhere it's necessary.
@@ -0,0 +1,21 @@ | |||
name: python-310 |
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.
Maybe an easier to understand name?
How about
name: python-310 | |
name: Sync python 3.10 docs |
?
Same for the other python-3*.yml files
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.
So far, this is the easiest solution to make easier to copy and paste the Transifex project name across the YAML files. Notice how I used ${{ github.workflow }}
as value below; this uses the 'name', which is 'python-310' in this case.
And it seems GitHub actions doesn't allow passing environment variable as input to reusable workflow (i tested it!), e.g.:
env:
PROJECT='python-310'
VERSION='3.10'
jobs:
sync:
uses: ./.github/workflows/sync.yml
with:
tx_project: ${{ env.PROJECT }}
version: ${{ env.VERSION }}
secrets: inherit
Co-authored-by: Vinicius Gubiani Ferreira <vini.g.fer@gmail.com>
Git output is already set to verbose
cd directly below will test already
Add version to the notification text
Este pull request visa mudar a scripts, workflows, etc. para atualização e teste das traduções para português brasileiro.
Vejo os seguintes problemas na solução atual:
A nova solução foi em muito baseada na do time python-docs-zh-cn, e tem as seguintes características:
main
como principal e, portanto, será de onde os workflows do GitHub Actions serão executados.scripts/
e será removido Makefile em favor de Shell e Python (com os quais tenho mais afinidade)./scripts/tx_stats.py
.A este pull request seguirá uma limpeza em cada branch de versão, de forma que cada branch de versão tenha apenas: