refatoracao e etc mt cansado pra uma mensagemzinha completa #2
Conversation
There was a problem hiding this comment.
Seu código ficou muito bom em alguns aspectos, deu pra ver que vc entendeu bem o conceito do negócio. Só tive algumas dúvidas em relação a tipagem das coisas que você realmente não tem o costume de fazer ou aparentemente não curte etc.
De resto, ce tá mandando d+ só vamo primo!
| @@ -60,7 +60,7 @@ TWITCH_OAUTH_SCOPES="user:read:email" | |||
|
|
|||
|
|
|||
| GITHUB_OAUTH_BASE_URI="https://id.twitch.tv/oauth2" | |||
| GITHUB_OAUTH_ID="" | |||
| GITHUB_OAUTH_ID="6b82a4205e7ce901190b" | |||
There was a problem hiding this comment.
ai namoral qq eu vo fazer com a sua chave? ce alterou o .env.example e nem viu menó toma cuidado com isso pq se vc tiver fazendo oss e uma key dessa passar ce vai tomar mt tapa na cara
There was a problem hiding this comment.
realmente nem fazia ideia q eu tinha mexido no .example kkkkkkkkkkkkkkk vacilo meu
|
|
||
| $user = $this->findOrCreate('github', $providerUser); | ||
| Auth::login($user); | ||
| $this->_repository->authenticate($provider, $code); |
There was a problem hiding this comment.
Me explica exatamente pra quê você meteu um _ ali. Fiquei curioso pra justificativa... Não sei se é só estética ou tem alguma coisa por trás.
There was a problem hiding this comment.
em outras linguagens onde nao da pra usar algumas keywords como public ou private (aka javascript), é normal colocar esse _ pra informar q o atributo em questao é privado ou q nao deve ser acessado por fora, isso aq é mais algo estético e de costume meu msm
| Auth::user()->update([ | ||
| 'image_path' => $imagePath | ||
| ]); | ||
| $this->_repository->postAvatar($request->file('image')); | ||
|
|
||
| return response()->json([], 200); |
There was a problem hiding this comment.
O que exatamente é 200? Pra nós, desenvolvedores um pouco mais experientes, sabemos que isso é um HTTP Code etc mas seria legal você usar os Enums ou classes estáticas como a Request pra colocar os HTTP Code como constante e ser mais legível.
| $users = User::orderByDesc('created_at')->paginate(4); | ||
| $registeredUsers = User::count(); | ||
| $messagesSent = Message::count(); | ||
| [ $users, $registeredUsers, $messagesSent ] = $this->_repository->getLandingContent(); |
There was a problem hiding this comment.
Eu achei meio estranho como isso aqui foi implementado, mas no final eu entendi o contexto.
Acho que pra um programador mais leigo seria um pouco difícil de entender, mas foi um bom pensamento.
|
|
||
| class AuthRepository { | ||
|
|
||
| public function authenticate($provider, $code) { |
There was a problem hiding this comment.
Você não acha interessante TIPAR as funções? Os argumentos e funções já são tipáveis tem uns belos anos dentro do PHP.
Fazendo isso, seu software vai ter mais garantia de que vai funcionar do jeito esperado.
| public function authenticate($provider, $code) { | |
| public function authenticate(string $provider, string $code): void | |
| { |
| use Illuminate\Support\Facades\Auth; | ||
| use Ramsey\Uuid\Uuid; | ||
|
|
||
| /* aqui fiquei em duvida se um nome melhor seria algo como userrepository porem pensei q poderia ser bem confuso caso esse projeto fosse ter um crud proprio pra os usuarios se registrarem etc */ |
There was a problem hiding this comment.
O nome tá ótimo, porém eu acho que se você subir um PR com uma descrição de código dessa pra PERGUNTAR algo você tomaria mt tapa na cara do seu chefe.
Coloca esse tipo de pergunta dentro da descrição do PR pelo amor de deus te amo.
| <a href="https://github.com/login/oauth/authorize?client_id={{env('GITHUB_OAUTH_ID')}}&redirect_uri={{env('GITHUB_REDIRECT_URI')}}&scopes={{env('GITHUB_OAUTH_SCOPES')}}" class="btn btn-primary"><i class="fab fa-github mr-5"></i> Sign in with Github</a> | ||
| <a href="https://id.twitch.tv/oauth2/authorize?client_id={{env('TWITCH_OAUTH_ID')}}&redirect_uri={{env('TWITCH_REDIRECT_URI')}}&response_type=code&scope={{env('TWITCH_OAUTH_SCOPES')}}" class="btn btn-purple"><i class="fab fa-twitch mr-5"></i> Sign in with Twitch</a> | ||
| @foreach (Config::get('providers') as $provider) | ||
| @if (!$provider['enabled']) |
There was a problem hiding this comment.
Primo, aqui você poderia ter jogado dentro da condição e já era. Usar o early return é bacana mas só quando você tem mais de uma condição/abstração.
O código teve +1 linha que não necessáriamente precisava.
|
|
||
| class UserRepository { | ||
|
|
||
| public function getLatestUsers() { |
There was a problem hiding this comment.
Por quê que uma função você tipou e a outra não? Hmmm...
@danielhe4rt