8000 remove the requirement to sort the dict file. by gilgamezh · Pull Request #360 · python/python-docs-es · GitHub
[go: up one dir, main page]
Skip to content

remove the requirement to sort the dict file. #360

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 24, 2020
Merged

remove the requirement to sort the dict file. #360

merged 2 commits into from
May 24, 2020

Conversation

gilgamezh
Copy link
Collaborator

Debido a que hemos sort del archivo del diccionario con el pre-commit esto esta creando un montón de conflictos a la hora de hacer merge de un nuevo PR.
Sí unicamente agregamos palabras nuevas al final del archivo esos conflictos van a producirse muy pocas veces.

  • eliminé el pre-commit que hace sort
  • agregué un chequeo en travis para buscar duplicados en el diccionario (va a fallar la primera vez)

gilgamezh added 2 commits May 24, 2020 21:23
@gilgamezh
remove the requirement to sort the dict file. It's creating conflicts…
f806547 
… in lot of PRs. And adding a check for duplicated entries
@gilgamezh
remove dups
50e9efd
@raulcd
Copy link
Collaborator
raulcd commented May 24, 2020

Qué bueno!!!

@raulcd raulcd merged commit 34a8ed3 into 3.8 May 24, 2020
@raulcd raulcd deleted the dont_sort_dict branch May 24, 2020 20:16
@humitos
Copy link
Collaborator
humitos commented May 24, 2020

Está bueno esto. Aunque tengo unas preguntas:

No entiendo porqué no habría conflictos si todos agregan palabras al final, cuando intente hacer merge va a notar que ambos modificaron la última línea y habrá conflicto igualmente, no?

Por otro lado, quizás no es buena idea hacer que el build falle por palabras duplicadas y bloquee el PR, ya que igualmente son ignoradas. ¿Que opinás?

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

@raulcd raulcd raulcd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gilgamezh @raulcd @humitos
0