Skip to content

refactor: update selic to v1.0.0#406

Merged
murilohns merged 1 commit intoBrasilAPI:mainfrom
caio-ribeiro-pereira:refactor/selic
Jul 13, 2022
Merged

refactor: update selic to v1.0.0#406
murilohns merged 1 commit intoBrasilAPI:mainfrom
caio-ribeiro-pereira:refactor/selic

Conversation

@caio-ribeiro-pereira
Copy link
Copy Markdown

Atualização do módulo selic para versão 1.0.0

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 26, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
brasilapi ✅ Ready (Inspect) Visit Preview Jul 13, 2022 at 6:11PM (UTC)

murilohns
murilohns previously approved these changes Jul 4, 2022
Copy link
Copy Markdown
Member

@LorhanSohaky LorhanSohaky left a comment

Choose a reason for hiding this comment

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

Só corrigiria o package-lock para manter na v1

import * as selic from 'selic';

const action = async (request, response) => {
const action = async (_request, response) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Não entendi o motivo de renomear, mas sem stress

Copy link
Copy Markdown
Author

@caio-ribeiro-pereira caio-ribeiro-pereira Jul 7, 2022

Choose a reason for hiding this comment

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

@LorhanSohaky o package-lock foi autogerado quando fiz um npm install para rodar a api localhost, inclusive o build aqui não passou por conta da v1, e com isso eu baixei e rodei a api local para atualizar o package-lock.
Sobre o renomear request para _request, provavelmente o lint + prettier do meu vscode, acabou corrigindo isso, pois nesse caso, request não esta sendo usado, porém por ele ser primeiro parâmetro é obrigatório carregar ele, e costuma ser uma boa prática em JS deixar com prefix _ variaveis que não são usadas mas que não da pra remover.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Entendi. Do request está tudo bem, só sobre o package-lock que é importante manter na V1. Você pode instalar uma versão um pouco mais antiga npx npm@6.14.17 i --save

Copy link
Copy Markdown
Author

@caio-ribeiro-pereira caio-ribeiro-pereira Jul 13, 2022

Choose a reason for hiding this comment

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

Blz, modificarei em breve, mas por curiosidade pq necessita manter v1 do package-lock.json? Pois o próprio Sonar deu problema quando subi do jeito antigo

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

É que seu PR contém alterações simples e que não estão diretamente ligadas ao package-lock. Quando o package-lock é modificado pode ser que ele atualize alguma dependência ou sub dependência, então nessas situações temos que olhar com mais atenção. Além disso, ao alterar o package-lock gerará conflito em todos so PRs abertos. Então as outras pessoas terão fazer rebase ou excluir o arquivo e instalar novamente

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@murilohns
Copy link
Copy Markdown
Collaborator

@caio-ribeiro-pereira fiz um Push no teu PR pra corrigir o package-json

Acontece que hoje fazemos o build do projeto na versão 12 do Node, e quando você deu o npm install, provavelmente estava na versão >= 14, então atualizou pra versão 2 do package-json.

Fiz um push na tua branch corrigindo isso, o resto tá certinho :)

Inclusive já está com meu Approve.

@caio-ribeiro-pereira
Copy link
Copy Markdown
Author

Obrigado pelo help @murilohns pois de fato, eu iria demorar fazer essa correção devido as correrias aqui!

@murilohns murilohns merged commit f35d6e2 into BrasilAPI:main Jul 13, 2022
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.

3 participants