search

CartoDB / carto-react-template

CARTO for React. The best way to develop Location Intelligence (LI) Apps usign CARTO platform and React
https://sample-app-react.carto.com
MIT License
39 stars 26 forks source link

Feature/ch131252/change config by store #196

Closed aaranadev closed 3 years ago

vercel[bot] commented 3 years ago

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/carto-frontend/cra-template-carto/kuywqgjs4
✅ Preview: https://cra-template-carto-git-feature-ch131252change-config-by-store.carto-frontend.vercel.app

shortcut-integration[bot] commented 3 years ago

This pull request has been linked to Clubhouse Story #131252: [Template] Add new hygen generators.

borja-munoz commented 3 years ago

I prefer "store" instead of "config".

From my point of view, a slice represents/manages a portion of the state tree and the store holds the state tree for the whole application.

It is not related just to configuration. I think it's more broadly related to what variables you are using to manage the state and how you update that state (store) in response to actions.

VictorVelarde commented 3 years ago

I'm ok with 'store' as that's the only configuration info we need now, for redux, and it's more detailed than a general 'config'. I think we should go ahead with the change

alasarr commented 3 years ago

It's not a strong opinion, go ahead if you prefer store

VictorVelarde commented 3 years ago

Sorry, I was trying to move this on, so I changed to master, merged latest changes there and checked again. But then I have seen that here there are some other changes, beyond the 'config' folder rename, so I'm creating a separated PR for the refactor.

VictorVelarde commented 3 years ago

Reverting PR name to original (to avoid misunderstandings with the new PR for the refactor)

VictorVelarde commented 3 years ago

This is the new PR, just for the refactor: https://github.com/CartoDB/carto-react-template/pull/201

VictorVelarde commented 3 years ago

This is not required anymore, as rename was already done in another PR