Skip to content
This repository was archived by the owner on Jul 11, 2026. It is now read-only.

Bren#13

Open
BrendaCarranco wants to merge 54 commits into
Laboratoria:masterfrom
Kari2406:bren
Open

Bren#13
BrendaCarranco wants to merge 54 commits into
Laboratoria:masterfrom
Kari2406:bren

Conversation

@BrendaCarranco

Copy link
Copy Markdown

No description provided.

Comment thread src/index.html Outdated
<i class="fas fa-search"></i>
</div>
<div class="filters-icon">
<a><i class="fas fa-filter"></i></a>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo con este código comentado, no se necesita entonces no se sube

Comment thread src/index.js Outdated
import { example } from './example.js';
n jm

//Crear usuario con email

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A menos que sea un bloque de código muy complejo que necesito un comentario entonces se pone, de otro modo sobra.

Comment thread src/index.js Outdated
$('#email-submit').click(function(){
let emailUser = document.querySelector('#email-new').value;
let passwordUser = document.querySelector('#password-new').value;
console.log(emailUser, passwordUser);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Se deben eliminar los console antes de hacer un push

Comment thread src/index.js Outdated

firebase.auth().createUserWithEmailAndPassword(emailUser, passwordUser)
.catch(function (error) {
// Errores

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comentario innecesario

Comment thread src/index.js Outdated
//invalidEmail.innerHTML =
//`
//<p>Correo ya registrado</p>
//`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Si es código es innecesario, debe ser eliminado

Comment thread src/index.js Outdated
//<p>Correo ya registrado</p>
//`
console.log(errorCode)
console.log(errorMessage)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo con los console

Comment thread src/index.js Outdated
});
});

//Ingresar usuario existente

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo aquí

Comment thread src/index.js Outdated
var photoURL = user.photoURL;
var isAnonymous = user.isAnonymous;
var uid = user.uid;
var providerData = user.providerData; */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Y aquí.

Comment thread src/index.html Outdated
</div>
</section>
<!-- <section class="menu bg-color">
</section> -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Código que no se utiliza se elimina antes de empujar.

Comment thread src/view-controller/router.js Outdated
case '#/follow':
case '#/menu':
case '#/newPost':

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por qué no hay un break después de cada case?

Comment thread src/view/follow.js Outdated
divElemt.classList.add('position')
divElemt.innerHTML = viewConexions;
return divElemt;
} No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Muy bien la indentación y orden de sus archivos! Felicidades por eso!

Comment thread src/view/home.js Outdated
`;

const divElemt = document.createElement('div');
divElemt.classList.add('position')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Te falta un ;

Comment thread src/view/menu.js Outdated
@@ -0,0 +1,10 @@
export default () => {
const viewConexions = `
<h1>hola</h1>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cuidar la indentación

Comment thread src/view/newPost.js Outdated
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="Close">
<span aria-hidden="true">&times;</span>
</button> <!-- boton cerrar -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Los comentarios son innecesarios

Comment thread src/view/newPost.js Outdated
`;

const divElemt = document.createElement('div');
divElemt.classList.add('position')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Falta un ;

Comment thread src/view/post.js Outdated
`;

const divElemt = document.createElement('div');
divElemt.classList.add('position')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Falta ;

Comment thread src/view/saved.js Outdated
`;

const divElemt = document.createElement('div');
divElemt.classList.add('position')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Falta un ;

Comment thread Firebase/app.js Outdated
)


//Add users

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Otro comentario innecesario

Comment thread Firebase/app.js Outdated
}


//Leer documentos

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo aquí y los espacios

Comment thread Firebase/index.html Outdated
<div id="root"></div>



Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hay muchos espacios innecesarios

Comment thread src/index.html Outdated
</div>
<div class="user-img">
<img src="img/xel.jpeg" alt="member">
</section> -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Código innecesario

Comment thread src/index.js Outdated
}

}
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Si no se necesita, debe eliminarse

Comment thread src/style.css Outdated

h3 {
text-align: center;
letter-spacing: 2px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cuidado con los selectores de tag

Comment thread src/firebase.js Outdated
});
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TODOS los comentarios que tienen son innecesarios

Comment thread src/firebase.js Outdated
var errorMessage = error.message;
console.log(errorMessage)
alert('Necesitas iniciar sesión para poder publicar un post.')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Los alert no son el mejor recurso. Si solo fueron una prueba se deben eliminar una vez que se empuje el código

Comment thread src/firebase.js Outdated
`;
let deletebutton = document.querySelectorAll('.js-delete');
let deletePost = (e) => {
console.log(e.target.id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Eliminar el console antes de hacer push

Comment thread src/firebase-auth.js
export const logOut = () => {
firebase.auth().signOut();
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Las funciones aqui declaradas son lo bastante explícitas como para poner un comentario que haga referencia de lo que hace cada una, son innecesarios

Comment thread test/example.spec.js Outdated
});
it('Debería poder iniciar sesion', () => emailLoginFb('grojasm@gmail.com', 'grojasm')
.then((user) => {
expect(user.email).toBe('grojasm@gmail.com');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Muy bien por los tests!

Comment thread test/example.spec.js Outdated
});
it('Debería poder iniciar sesion', () => emailLoginFb('grojasm@gmail.com', 'grojasm')
.then((user) => {
expect(user.email).toBe('grojasm@gmail.com');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Súper bien!

@AdrianaHY AdrianaHY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Chicas, las felicito por su buen trabajo y avance. Muy bien que se hayan metido ya a hacer testing.

Sin embargo hay cosas en las que debemos seguir trabajando, las buenas prácticas es algo que seguimos dejando de lado, los comentarios, espacios y console son innecesarios y deben eliminarse antes de hacer un push.

Eviten usar en la medida de lo posible los important, entiendo que por el framework que se utilizó quizá era la solución en el momento, pero busquen hacerlo de otra manera.

Felicidades por su gran esfuerzo!!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants