search

Ada-Online-2da-Gen / Todo-App

Repositorio para el proyecto colaborativo de Todo App de Ada Online 2da Generación
2 stars 0 forks source link

Issue #1 - Agregar ítem a la lista y borrar input #32

Closed LoohanZero closed 4 years ago

LoohanZero commented 4 years ago

Agrego la funcionalidad de generar un ítem de la lista, que se borre el value del input después de un click en el botón o un enter y agrego placeholder. Espero comentarios.

LoohanZero commented 4 years ago

Cuando tengas que mergear, no arregles el conflicto solo porque queremos ver cómo lo hacés en clase. =D

pabloHoc commented 4 years ago

Otro detalle, a nivel de nombre de PR mejor si aclarás de que va el PR, así cuando veo la lista de todos los que hay puedo identificarlos claramente, p. ej.:

Cierro #1 - Agregar todo

LoohanZero commented 4 years ago

Genial Lu! Buen primer PR 💪 Ahí te dejé algunas cuestiones para que mires, son más que nada temas de estilo, la lógica está bien.

Pregunta, hiciste el git rebase origin/master en tu branch? Porque no tenés lo último que agregué, por eso te está tirando el conflicto.

Lo hice, y me tiró que no había nada que rebasear, por eso empecé a trabajar normal.

LoohanZero commented 4 years ago

Varias preguntas que tarde pero seguro: ¿debería hacer un commit por cada corrección que me hacés? Con lo del rebase, me sigue tirando que tengo la branch y la master up to date. Me aparece arriba el "shortID" pero en la master, no en mi branch, por ahí es por eso. Ah, y otra pregunta que me quedó pendiente, ¿estos otros componentes que estoy haciendo (handleClick y handleKeyPress) que llaman a addTodo, debería hacerlos en archivos separados o está bien que estén también dentro de App?

pabloHoc commented 4 years ago

Hoy vemos lo del rebase. Con lo del commit, buena pregunta, no es un criterio tan estricto el que decís, pero sí uno para tener en cuenta. La idea de un commit es que representa un "cambio" como concepto, si bien es una distinción muy subjetiva, la idea es que implica una cierta unidad. En este caso todos los fix los podrías meter en un commit porque son cuestiones de estilos sobre un mismo componente, no hay una funcionalidad ni nada nuevo que se agregue o elimine.

Las funciones por ahora están bien en App, pero después va a haber que moverlas, lo dejo como un issue nuevo cuando sea necesario 😉