Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Github Action para ejecutar powrap fix con un comentario#1836

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
erickisos wants to merge35 commits intopython:3.13
base:3.13
Choose a base branch
Loading
fromerickisos:3.10

Conversation

erickisos
Copy link
Contributor

@erickisoserickisos commentedOct 5, 2022
edited by humitos
Loading

Descripción

Esta es la versión probada de la Github Action para ejecutar pospell con un comentario.

Uso

Los pasos del workflow son los siguientes:

  • Comenta en un PRpospell-fix
  • El Github Bot reaccionará con un 👍 a tu comentario, para notificarte que se está ejecutando.
  • Si tras ejecutarpowrap ypospell el bot encuentra cambios, se generará un commit directamente a tu PR.

Referencias

erickisosand others added18 commitsFebruary 13, 2022 12:01
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
Add Github action to run pospell on a comment
@erickisoserickisos changed the titlePospell-fix Github ActionGithub Action para ejecutar pospell fix con un comentarioOct 5, 2022
Este cambio no está relacionado al PR, así que no hace sentido tenerlo acá.
@erickisos
Copy link
ContributorAuthor

Acá está una versión que saqué después de probar en mi fork@rtobar@ezio-melotti 🥳

@rtobar
Copy link
Collaborator

pospell-fix

run: python -m pip install -r requirements.txt
- name: Ejecutar Powrap
run: powrap --quiet **/*.po
- name: Revisar con Pospell
Copy link
Collaborator

Choose a reason for hiding this comment

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

Como mencionaba en el otro PR, pospell no hace ningún cambio local en tus archivos, sólo chequea si la ortografía es correcta o no, y retorna con status 0 o distinto a 0 dependiendo de si encontró errores. Esto significa que hacer chequeo ortográfico no tiene incidencia alguna en el paso siguiente (hacer un commit con los cambios locales), por lo que encuentro que no tiene mucho sentido ejecutar pospell en primer lugar -- aparte que hace que la acción se demore más en terminar.

En mi opinión la acción debería estar centrada solo en ejecutar powrap y hacer el commit con los cambios que este encuentre, nada más (al menos esa era mi intención al crear#1786). Por eso también insistía en que los nombres del archivo, de la acción, etc, estuvieran todos centrados en powrap, no en pospell.

erickisos reacted with heart emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Oh vaya, got it, déjame hacer esos cambios de una, la verdad no me había quedado claro 😅

Copy link
Member

@ezio-melottiezio-melotti left a comment

Choose a reason for hiding this comment

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

Algunas sugerencias sobre un nuevo comandopospell-check que solo dice si la PR es buena sin hacer el commit.
Tambien podrias ejecutar el check automaticamente cuando la PR es creada/modificada, y solo haber el comandopospell-fix para hacer el commit.

run: python scripts/check_spell.py
continue-on-error: true
- name: Commit & Push changes
uses: actions-js/push@master
Copy link
Member

Choose a reason for hiding this comment

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

Tambien hayesta action que parece hacer el mismo, pero en el marketplace tiene 1k star (actions-js/push solo tiene 38).
Nunca las he probado, así que no puedo decirte cuál es la mejor.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Hice el cambio solo para hacer la prueba, si vemos que no nos funciona as expected, le damos de vuelta a la que teníamos

ezio-melotti reacted with thumbs up emoji
@erickisoserickisos requested review fromrtobar andezio-melotti and removed request forrtobarOctober 5, 2022 04:40
@erickisos
Copy link
ContributorAuthor

Ya hice varios de los cambios sugeridos, pero lamentablemente creo que no podemos ejecutarlo hasta que le demos merge en este repo 😞

De todas maneras por si las moscas, voy a crear un PR en mi fork, para probar la ejecución.
@rtobar

@erickisoserickisos requested review fromrtobar and removed request forezio-melottiOctober 5, 2022 04:44
@erickisos
Copy link
ContributorAuthor

Tras los cambios, en este PR (erickisos#6) ejecuté el comando y podemos ver los resultadosacá

rtobar reacted with thumbs up emoji

@erickisos
Copy link
ContributorAuthor

Desafortunadamente me he dado cuenta de que el commit action no encuentra cambios tras ejecutar powrap, no estoy seguro de por qué 😢

@erickisoserickisos changed the titleGithub Action para ejecutar pospell fix con un comentarioGithub Action para ejecutar powrap fix con un comentarioOct 8, 2022
@cmaureircmaureir added the hacktoberfest-acceptedmantainer-approved contribution to hacktoberfest labelOct 25, 2022
Copy link
Collaborator

@rtobarrtobar left a comment

Choose a reason for hiding this comment

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

Perdón@erickisos por tomarme tanto tiempo en responder pero andaba de vacaciones y sin mucho tiempo para ver otros temas.

Veo que la mayoría de los comentarios ya los tomaste en cuenta, y la acción ahora sólo ejecutapowrap como era la intención original. Estuve viendo los ejemplos de ejecución que pusiste, y otros más, y encontré otro par de problemas que hay que arreglar, pero en general ser está viendo súper bien 👍

@humitos
Copy link
Collaborator

En#1786 (comment) comenté porqué creo que esto puede ser un problema, y propuse una solución "inofensiva" a primera vista.

@erickisos
Copy link
ContributorAuthor

Hey, hola a todos, le di ya una revisada a los comments, pero me parece super interesante el que menciona@humitos por acá (#1786), Qué opinan ustedes?

@rtobar@ezio-melotti

@github-actions
Copy link

Este PR lleva un tiempo sin actualizaciones. Vamos a pedir a un admin de nuestro equipo que decida si alguien más puede finalizarlo o si tenemos que cerrarlo.\nPor favor, avisanos en caso de que aún puedas terminarlo.

@erickisos
Copy link
ContributorAuthor

Estaría bueno revisar si podemos etiquetar gente con el stale bot, supongo que después

@cmaureircmaureir changed the base branch from3.10 to3.13November 21, 2024 20:49
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@rtobarrtobarrtobar left review comments

@ezio-melottiezio-melottiezio-melotti left review comments

Assignees
No one assigned
Labels
hacktoberfest-acceptedmantainer-approved contribution to hacktoberfest
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@erickisos@rtobar@humitos@ezio-melotti@cmaureir

[8]ページ先頭

©2009-2025 Movatter.jp