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

Feature: Allow Channel ID's to be overridden via .env #582

Closed
3 of 4 tasks
Asartea opened this issue Aug 13, 2024 · 1 comment · Fixed by #584
Closed
3 of 4 tasks

Feature: Allow Channel ID's to be overridden via .env #582

Asartea opened this issue Aug 13, 2024 · 1 comment · Fixed by #584
Assignees
Labels
Status: In Progress This issue/PR has ongoing work being done

Comments

@Asartea
Copy link
Contributor

Asartea commented Aug 13, 2024

Complete the following REQUIRED checkboxes:

  • I have thoroughly read and understand The Odin Project Contributing Guide

  • The title of this issue follows the command name: brief description of request format, e.g. /help: add optional @user parameter

The following checkbox is OPTIONAL:

  • I would like to be assigned this issue to work on it

1. Description of the Feature Request:
Currently, all of the channel ID's are stored within config.js. This means that if a contributor want to test some piece of code that depends on a specific ID, the only way to do so in a personal server is to change the value there manually.

The issue with this is that config.js is a git tracked file, which means that if they forget to do undo their change before committing their changes (this is especially dangerous when you are also making an actual config change), or risk breaking production.

I propose specifying all channel ID's instead in the following format

channelNameChannelId: process.env.CHANNEL_NAME_CHANNEL_ID || <actual id>

This allows the ID to be changed by setting the right env variable in .env instead, which is kept only locally and doesn't require the make change, test, remember to undo change, forget, commit, frantically undo change, commit, force push, hope nobody merged the PR cycle.

2. Acceptance Criteria:

  • All channel ID's are specified in the above format

3. Additional Information:

@Asartea Asartea added the Status: Needs Review This issue/PR needs an initial or additional review label Aug 13, 2024
@MaoShizhong
Copy link
Contributor

This makes sense - assigned.
You'll have to decide on the format for the env file for config fields containing arrays.

@MaoShizhong MaoShizhong added Status: In Progress This issue/PR has ongoing work being done and removed Status: Needs Review This issue/PR needs an initial or additional review labels Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress This issue/PR has ongoing work being done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants