search

hackforla / 311-data

Empowering Neighborhood Associations to improve the analysis of their initiatives using 311 data
https://hackforla.github.io/311-data/
GNU General Public License v3.0
62 stars 63 forks source link

Modify CheckEnv.js to notify dev if Vite keys are missing from .env file #1824

Open aqandrew opened 1 month ago

aqandrew commented 1 month ago

Overview

We need to modify our check-env script to notify the dev if they are using non-vite compatible environment variables so that we can ensure previous devs have migrated to Vite.

Action Items

Resources

Info about relevant files

Archived Info

Details

> > ### Overview > > This change is a simple variable renaming, which must be done for everyone running 311-Data after we switch from webpack to Vite. It could be done manually by inserting `VITE_` at the beginning of their .env file. Or we could automate it, for example by modifying [checkEnv.js](https://github.com/hackforla/311-data/blob/d68ff874b7433344d26fe0f92865a9502e700cc8/utils/checkEnv.js). > > Additionally, the repository secret must be updated so that the Mapbox API key is available in GitHub Actions. See https://github.com/hackforla/311-data/issues/1778#issuecomment-2316193782 > > ### Action Items > - [x] determine the best method for renaming this variable for all 311-Data developers (manual or using a script) > - [ ] ensure that this variable is renamed for everyone by the time #1822 is merged > - [ ] add/update the correctly-named repository secret by the time #1822 is merged > > ### Resources/Instructions > None

traycn commented 1 month ago

Reviewed PR #1822 dependency and it's looking great. Appreciate the clean up in the DbProvider.

traycn commented 1 month ago

For existing developers, modifying the checkEnv.js file sounds like a great idea. It'd be nice to take into account those previous devs who aren't aware of this change too.

For new developers, we can update the .example.env file and pre-pend VITE_ for the env variables there. So that cp .example.env .env reflect these changes. Refering to the command in the (README.md) steps.

aqandrew commented 3 weeks ago

Updating .example.env is a great idea. I'll do that + update/test checkEnv.js and include them in the PR.

Since checkEnv.js isn't mentioned in the README, I'll add some instructions for it there as well.

ryanfchase commented 3 weeks ago

@traycn @aqandrew I'm going to modify the ticket in a couple of different ways:

  1. dev leads are going to take care of this in a separate issue:

    • [ ] ensure that this variable is renamed for everyone (they should all modify .env file since this is not checked into git)

  2. regarding automation of checking VITE_ prefix, we will make a dev ticket to update checkEnv.js to throw error (e.g. "Vite prefix needed in .env file")

    • [ ] I'd like to change the title / overview of this ticket to accomplish this change
ryanfchase commented 3 weeks ago

This ticket is now dedicated to modifying the CheckEnv.js file. Changes to .example.env will be done on this ticket's PR

ryanfchase commented 3 weeks ago

After discussing this ticket, we've decided this work should be done as part of the existing PR. @aqandrew will get started on this, the work needs to be completed before the PR is merged.

ryanfchase commented 2 weeks ago

Moved to In Review since I've been re-requested on the PR.