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

utilize process.env #91

Merged
merged 1 commit into from
Nov 29, 2024
Merged

utilize process.env #91

merged 1 commit into from
Nov 29, 2024

Conversation

zacuke
Copy link
Contributor

@zacuke zacuke commented Nov 29, 2024

The trick is process.env only works if the name starts with REACT_APP

Base .env still has REACT_API_URL since it's used in docker scripts.

This could be cleaned up a little bit more by also updating docker scripts to use the longer REACT_APP_API_URL version of the env var, but this was got it to where I don't have an extra changed file axiosInstance.js in my working folder.

I also updated documentation a little bit. There is a thing to prevent package.json from changing every time yarn start is launched. There's a lively discussion regarding what is happening there. You might not be experiencing that if your nodejs or npm is older.

I hope my PR are beneficial. I'm still getting familiar with the project.

@r0fls
Copy link
Owner

r0fls commented Nov 29, 2024

I hope my PR are beneficial. I'm still getting familiar with the project.

More than beneficial, it's super encouraging to have someone else take a more in-depth look at this project with me. Also FWIW, I looked at the previous issue you fixed a few times and couldn't figure it out, it had still been bothering me. This one too I had meant to get to, but hadn't had the time. So, beyond appreciating your interest an participation in general, these things are real problems I needed help with.

Anyway, please open an issue if you experience any other problems or something doesn't make sense: a lot of the decisions were made with the intent to go back and enhance/complete them, or refactor/redesign altogether. That doesn't always happen though. Also, definitely still a WIP project, though I have been running this live to make trades for about 4 or 5 months now.

Also, if you continue to onboard to the project we can potentially setup a sync of some sort to discuss some of my other goals/plans, in case you want to help out with any of the future enhancements I already have sketched out. Or, there is plenty of cleanup work that needs to happen as well, as you're finding. If you want to set something up feel free to email me at [email protected].

Sorry, lot's of words but I was just really stoked to see your PRs come through! 😄 Thanks again 🙏

@r0fls r0fls merged commit a10ee05 into r0fls:main Nov 29, 2024
3 checks passed
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.

2 participants