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

Add custom_dc/env.list to .gitignore #4573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wittrock
Copy link
Contributor

The instructions for running a custom datacommons include creating a file (custom_dc/env.list) which is likely to contain API keys. Add it to .gitignore to avoid accidental checkins.

@wittrock wittrock changed the title [gitignore] Add custom_dc/env.list Some setup fixes Aug 23, 2024
@dwnoble
Copy link
Contributor

dwnoble commented Aug 23, 2024

Hey @wittrock , I like this change to avoid accidentally checking in API keys, but i think it's also helpful to have a template of what variables are available. What do you think about renaming env.list to env.list.sample and ignoring (like you have) env.list?

Then we'd need to update the instructions here: https://docs.datacommons.org/custom_dc/custom_data.html (https://github.com/datacommonsorg/docsite) to tell the user to copy the env.list.sample file to the env.list file

wdyt @keyurva @kmoscoe

@keyurva
Copy link
Contributor

keyurva commented Aug 23, 2024

I like Dan's idea. To not break existing users, can we do it in 3 steps:

  1. Create a PR with env.list duplicated as env.list.sample.
  2. Update the doc that uses the latter.
  3. Create a PR that removes env.list and adds it to .gitignore.

Thoughts?

@kmoscoe
Copy link
Contributor

kmoscoe commented Aug 23, 2024 via email

@kmoscoe
Copy link
Contributor

kmoscoe commented Aug 23, 2024 via email

@wittrock wittrock changed the title Some setup fixes Add custom_dc/env.list to .gitignore Aug 23, 2024
@wittrock
Copy link
Contributor Author

Done, created :

  1. [custom_dc] Add env.list.sample #4577 to add env.list.sample and
  2. [custom_dc] Update instructions to refer to env.list.sample docsite#488 to update the docs

Those two should merge first, then this one.

@wittrock
Copy link
Contributor Author

Also split out my setup instructions change into #4578

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.

4 participants