Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Review #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Code Review #1

wants to merge 1 commit into from

Conversation

fabiolmorato
Copy link

No description provided.

@vercel
Copy link

vercel bot commented Nov 5, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/thaliadettenborn/linkr/27fwv6p1p
✅ Preview: Failed

Copy link
Author

@fabiolmorato fabiolmorato left a comment

Choose a reason for hiding this comment

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

O projeto ficou bem escrito, organizado e legível. Levantei alguns pontos por comentários no Pull Request. Destaco que, embora tenham sido feitos em partes específicas do código, valem para ele todo. Em geral, o grupo mostrou domínio dos conceitos apresentados em aula.

width: auto;
}
}
`;
Copy link
Author

Choose a reason for hiding this comment

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

Boa parte desse arquivo seguiu indentação com dois espaços enquanto outras 4. O que aconteceu?

const dataPost = {"link": link.value, "text": text.value};

axios
.post("https://mock-api.bootcamp.respondeai.com.br/api/v1/linkr/posts",dataPost,{headers: tokenUsuario})
Copy link
Author

Choose a reason for hiding this comment

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

Poderia haver espaços entre os argumentos da função, deixaria o código ainda mais legível :)


export default function FormLogin(props) {
const {register, setRegister} = props;
const {SubmitLogIn,SubmitSingUp,sendRequest,setSendRequest} = useContext(UserContext);
Copy link
Author

Choose a reason for hiding this comment

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

Também poderia haver espaços nesse destructuring, melhoraria a legibilidade desse trecho.


function handleLoader(){
setHasMore(false);
setOffset(offset+10);
Copy link
Author

Choose a reason for hiding this comment

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

Também poderia haver espaço aqui, offset + 10. Fica mais legível.

height: 100vh;
margin: 105px 30% 0 auto;

@media (max-width: 700px){
Copy link
Author

Choose a reason for hiding this comment

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

Esses @media poderiam ser armazenados em um arquivo de definições JS, uma vez que aparecem em mais de um arquivo. Se algum dia for tomada a decisão de modificar o valor, vários lugares devem ser alterados.

const [sendRequest,setSendRequest] = useState(false);
const history = useHistory();

function SubmitLogIn(event){
Copy link
Author

Choose a reason for hiding this comment

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

Nomes de funções deveriam começar com letras minúsculas. Somente as funções que são componentes deveriam começar com letra maiúscula. Isso facilita a leitura do código, uma vez que pode-se assumir se é um componente ou função pelo nome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant