Skip to content
This repository was archived by the owner on Feb 16, 2021. It is now read-only.

Fixing dist actions/core dependency#9

Draft
igraguiar wants to merge 3 commits intomainfrom
fix/add-path-error
Draft

Fixing dist actions/core dependency#9
igraguiar wants to merge 3 commits intomainfrom
fix/add-path-error

Conversation

@igraguiar
Copy link

Recompiled main.ts to fix code dependencies.

@igraguiar igraguiar added the bug Something isn't working label Jan 10, 2021
@igraguiar igraguiar self-assigned this Jan 10, 2021
@gullitmiranda gullitmiranda self-requested a review January 27, 2021 02:11
Copy link
Member

@gullitmiranda gullitmiranda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igraguiar esse PR ainda está como Draft, mas vou deixar o review para podermos finalizar isso.

Os pontos que precisa ser visto são:

  1. Atualizar o @actions/core que é o motivo do ter deixado de funcionar (https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/)

  2. O artefato expira em 90 dias, sendo assim precisamos encontrar uma alternativa, as sugestões seriam

a. Mais simples: adicionar um schedule para o workflow test.yml para ser executado com uma frequência menor que 90 dias
b. Ideal: Fazer o upload do artefato para releases.

Sobre o 2.b, se eu não estou enganado o problema era com o arquivo compactado no release. Ao extrair ele perdia as referências necessárias

  1. se não for muito trabalho, já fazer a atualização das dependências vulneráveis: https://github.com/yubeio/setup-imagemagick/security/dependabot

Copy link
Member

@gullitmiranda gullitmiranda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igraguiar levantei um ponto nos comentários.

Além disso, com as mudanças no PR https://github.com/yubeio/hive/pull/424 isso não vai ser mais necessário.

Utilizar o ghostscript resolve tanto a questão de precisar do imagemagick compilado a partir do source quanto as miniaturas estarem sendo gerada erradas.

# Required, uploaded artifact name
name: ImageMagick-${{ env.IMAGEMAGICK_VERSION }}-x64-precompiled
# Optional, directory where to extract artifact
path: ../ImageMagick-${{ env.IMAGEMAGICK_VERSION }}-x64-precompiled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igraguiar será que faz sentido ativar isso para o workflow final?

pq se isso tiver ativado o build pode não acontecer como esperado, já que ele pode usar o cache.
A ideia de ter isso comentado era para poder pular o processo de compilação enquanto estivesse trabalhando nos outros steps.

Copy link
Author

@igraguiar igraguiar Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eu pensei em usar pois se a versão desejada não mudar, em teoria não haveria problema em usar a versão do cache. Já que também temos a etapa que testa se o artefato funciona conforme o desejado. @gullitmiranda Nós vamos continuar com a correção desse projeto? Como você mencionou no slack que talvez não precisaríamos e vendo as alterações no ci/cd do Hive acabei não retomando aqui.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eu pensei em usar pois se a versão desejada não mudar, em teoria não haveria problema em usar a versão do cache
vc está certo @igraguiar, faz bastante sentido

@gullitmiranda Nós vamos continuar com a correção desse projeto? Como você mencionou no slack que talvez não precisaríamos e vendo as alterações no ci/cd do Hive acabei não retomando aqui.

Não precisamos mais @igraguiar. pode deixar isso para lá. Se tivesse pronto, valeria a pena fazer merge.
Mas como não está, não faz mais sentido investir nisso agora

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments