digidem / mapeo-mobile

Monitor and document the world around you
GNU General Public License v3.0
95 stars 16 forks source link

fix: adjust mapbox access token usage #1150

Closed achou11 closed 3 months ago

achou11 commented 3 months ago
ErikSin commented 3 months ago

I did a manual build in bitrise to test the use of the secret key and everything seems to be working: https://app.bitrise.io/build/5ee6fbee-1cb2-4924-9eaa-8b06619035c4?tab=artifacts

ErikSin commented 3 months ago

@gmaclennan Im currently using a token from my personal account. Were you thinking that we just use a free token from the Digidem account, or did you have something else in mind?

ErikSin commented 3 months ago

Updated the token (to the digidem account token) in bitrise, and the reference to the encrypted token in the cirrus.yml

gmaclennan commented 3 months ago

FYI process.env doesn't really exist in the react native runtime

achou11 commented 3 months ago

FYI process.env doesn't really exist in the react native runtime

this was done at my suggestion to make it slightly easier for local development by taking advantage of our usage of https://babeljs.io/docs/babel-plugin-transform-inline-environment-variables (being used for feature flagging). can revert it and instead update the docs to say that an env file is now required if you think that's a better way to go

gmaclennan commented 3 months ago

Ah great, didn't realise we used that in comapeo. We could have skipped adding react-native-config 😀

ErikSin commented 3 months ago

Ah great, didn't realise we used that in comapeo. We could have skipped adding react-native-config 😀

I don't think its working for .env files. It seems like it is only picking up shell env variables. So react-native-config is still needed. There was some work we could have done to update babel and make it work, but that lead to a whole slew of other updating issues

achou11 commented 3 months ago

Ah great, didn't realise we used that in comapeo. We could have skipped adding react-native-config 😀

yeah unfortunately it would be a little inconvenient to only rely on this for local development as you have to specify/export the environment variable in the shell for the command that eventually runs react-native bundle, so we'd still probably need some kind of env file to work with anyways, unless we hardcoded the access token in a few places 😄

achou11 commented 3 months ago

decided to revert the change introducing process.env usage and just make it required to set up a .env file for local development. i updated the contributing docs to be a little clearer about what's needed for that part