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

Readme.md #4

Merged
merged 13 commits into from Dec 28, 2022
Merged

Readme.md #4

merged 13 commits into from Dec 28, 2022

Conversation

ghost
Copy link

@ghost ghost commented Dec 26, 2022

Criei instruções para a instalação do projeto na maquina local de quem queira contribuir.

Adicionado comandos de npm

Copy link
Member

@IvoPereira IvoPereira left a comment

Choose a reason for hiding this comment

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

Hey Sérgio! Obrigado pela contribuição 👍

Fiz uns pequenos ajustes mais para uniformizar a pessoa na qual se explica a documentação. Diz-me o que achas e caso não concordes com algo.

Idealmente seria interessante depois termos dividido por pequenas categorias (Instalação, workflow de desenvolvimento, boas práticas, contribuição, etc), mas por agora um pequeno README.md com o que enviaste acho que já ajuda imenso. Bom trabalho! 👍

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sergiofresco and others added 5 commits December 27, 2022 09:30
Co-authored-by: Ivo Pereira <ivoecpereira@gmail.com>
Co-authored-by: Ivo Pereira <ivoecpereira@gmail.com>
Co-authored-by: Ivo Pereira <ivoecpereira@gmail.com>
Co-authored-by: Ivo Pereira <ivoecpereira@gmail.com>
Co-authored-by: Ivo Pereira <ivoecpereira@gmail.com>
@IvoPereira IvoPereira added the documentation Improvements or additions to documentation label Dec 27, 2022
@ijpatricio
Copy link
Collaborator

Olá @sergiofresco ! Mt obrigado pela disponibilidade.
Deixei uma review em alguns pontos, validas, pf?
Thanks!

@ghost
Copy link
Author

ghost commented Dec 27, 2022

Huh, @ijpatricio ajuda-me. Onde posso ver a review? :S

@ghost ghost changed the title Readme.md Readme.md and NavBar Dec 28, 2022
shenriques added 3 commits December 28, 2022 15:02
@ghost ghost changed the title Readme.md and NavBar Readme.md Dec 28, 2022
Copy link
Member

@IvoPereira IvoPereira left a comment

Choose a reason for hiding this comment

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

Fora os comments que me enviei só retirava os commits referentes à Navbar e enviava no respetivo PR.

Um dos objetivos do repo e melhores práticas que queremos seguir inclui também enviarmos apenas as alterações referentes ao escopo do PR.

Achas que conseguias?

Fora isso e os comments que enviei acima parece-me ok 👍 alguma sugestão @ijpatricio?

Comment on lines +60 to +61
Certifica-te que tens o docker daemon a correr antes de lançar o servidor pelo
laravel sail. Para teres o daemon a correr manualmente:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Certifica-te que tens o docker daemon a correr antes de lançar o servidor pelo
laravel sail. Para teres o daemon a correr manualmente:
Certifica-te que tens o docker daemon a correr antes de lançar o servidor pelo Laravel Sail. Para teres o daemon a correr manualmente:

Comment on lines +51 to +52
Adiciona a propriedade APP_PORT para mudares a porta se a porta 80 já estiver
a ser usada. Aqui usamos 8080 por exemplo mas pode ser outra.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Adiciona a propriedade APP_PORT para mudares a porta se a porta 80 já estiver
a ser usada. Aqui usamos 8080 por exemplo mas pode ser outra.
Adiciona a propriedade APP_PORT para mudares a porta se a porta 80 já estiver a ser usada. No projeto usamos 8080 por exemplo mas pode ser outra

npm install
```

Duplicar o ficheiro `.env.example` para `.env` e alterar os dados se necessário.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Duplicar o ficheiro `.env.example` para `.env` e alterar os dados se necessário.
Duplicar o ficheiro `.env.example` para `.env` e alterar os dados se necessário

@ijpatricio
Copy link
Collaborator

Concordo @IvoPereira , o PR ficou com scope creep 😄

No entanto, podemos assumir o merge, uma vez que assumo eu fazer as afinações. Faço isto para que o @sergiofresco tb não se desmotive. para meter alterações a Front e Back num PR de README.md é porque não está habituado, e pronto, está com um tamanho que assumo fazer o "reconcile". 👍

Não vamos poder fazer sempre disto, claro! 😄

@IvoPereira
Copy link
Member

No entanto, podemos assumir o merge, uma vez que assumo eu fazer as afinações.

Nesse caso @sergiofresco vamos aceitar o merge assim e o @ijpatricio assim que possível faz as afinações necessárias 😄

Bom trabalho 👍

@IvoPereira IvoPereira merged commit 8353c78 into devpt-org:main Dec 28, 2022
@ghost ghost mentioned this pull request Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants