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

Development setup #62

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Development setup #62

wants to merge 34 commits into from

Conversation

lguima
Copy link
Contributor

@lguima lguima commented Feb 24, 2022

Setting up some tools to improve the development across contributors.

Changelog

  • Add EditorConfig
  • Add ESLint
  • Add Prettier
  • Remove debug
  • Move images
  • Update README
  • Create CONTRIBUTING file
  • Add Husky
  • Add lint-staged

@vercel
Copy link

vercel bot commented Feb 24, 2022

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/cmdalbem/ciclomapa/aiJsxS6tfuz3aaTkGPqnmFBpZxTN
✅ Preview: https://ciclomapa-git-fork-lguima-development-setup-cmdalbem.vercel.app

@lguima
Copy link
Contributor Author

lguima commented Feb 28, 2022

Now the build it's working :)

Since I had to handle the issues raised by ESLint, I also added the Husky to validate them when making a commit.

@@ -163,8 +163,6 @@ class App extends Component {
}
})

console.debug('url params obj:', paramsObj);
Copy link
Owner

Choose a reason for hiding this comment

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

oi @lguima !
alguma razão pra ter removido essa linha de debug, ou foi por acidente? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Achei que era um debug esquecido, é proposital? hehe

@@ -0,0 +1,49 @@
# Contributing
Copy link
Owner

Choose a reason for hiding this comment

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

boa, brigado por organizar esse arquivo!
tava há tempos querendo fazer isso haha

@@ -3,398 +3,419 @@ import { Popover, Button } from 'antd';

import { PieChart, Pie } from 'recharts';

import {
Copy link
Owner

Choose a reason for hiding this comment

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

só de curiosidade, todas essas alterações foram feitas automaticamente pelas ferramentas, ou algumas vc teve que fazer na mão?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Teve algumas que foi preciso fazer na mão, depois foi o autofix.

src/OSMController.js

src/MapPopups.js

src/AnalyticsSidebar.js

}

Copy link
Owner

Choose a reason for hiding this comment

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

vc chegou a reordenar ou refator algo?
ou foi só o diff do github que se perdeu aqui?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Foi uma correção de regra que estava reclamando e autofix.

src/AnalyticsSidebar.js

} else {
fetch(`https://nominatim.openstreetmap.org/search?format=json&q=${encodeURI(areaName)}`)
Copy link
Owner

Choose a reason for hiding this comment

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

só comentando, esse aqui é um exemplo de como o eslint, as vezes, piora a legibilidade do código hehe não achas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ficou assim, na vdd:

fetch(
  `https://nominatim.openstreetmap.org/search?format=json&q=${encodeURI(
    areaName
  )}`
)

É possível ignorar a linha para não ser formatada, se for o caso.

Acho que tem mais casos de melhoria do que de piora, não? Já havia sido considerado o linter?

...
"editor.formatOnSave": false,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
Copy link
Owner

Choose a reason for hiding this comment

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

essa configuração faz o editor corrigir automaticamente todos warnings quando o arquivo é salvo, certo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exatamente, assim garante que quando for fazer o commit, já está corrigido.

@@ -30,11 +30,18 @@
},
"scripts": {
"start": "craco start",
"lint": "eslint src --max-warnings=0",
Copy link
Owner

Choose a reason for hiding this comment

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

essa configuração obriga a ter zero warnings de eslint, certo?
vc se importa se não usarmos essa configuração? pessoalmente as vezes eu tenho preferencia por, em alguns casos muito específicos, quebrar algumas regrinhas pra melhorar a legibilidade do código

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nesses casos tem como ignorar o linter para determinados arquivos ou linhas, assim não gera warning:

@vercel
Copy link

vercel bot commented May 21, 2022

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

Name Status Preview Updated
ciclomapa ✅ Ready (Inspect) Visit Preview May 21, 2022 at 7:17PM (UTC)

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.

2 participants