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

fix: delete invalid envs #336

Merged
merged 2 commits into from
Jul 11, 2024
Merged

Conversation

dawsbot
Copy link
Contributor

@dawsbot dawsbot commented Jul 5, 2024

It seems that the only valid env in the filesystem is .env.bitmama. For ease of onboarding for folks (like myself), the invalid envs should be fixed or removed entirely. Here is the error I was getting:

yarn jest validations/clock.test.ts

...
getaddrinfo ENOTFOUND fiat-connect.sandbox.alphafortress.com
...

@silasbw
Copy link
Member

silasbw commented Jul 7, 2024

@satish-ravi can you provide feedback?

@dawsbot
Copy link
Contributor Author

dawsbot commented Jul 8, 2024

It looks like a ton of prior partners or folks who had implemented an API have either changed or shut down the URLs which are placed in those envs that I removed @silasbw. My hunch would be that fixing them is preferred to removing them, but I don't have those relationships to check-in with those partners.

@satish-ravi
Copy link
Contributor

I think we can remove everything except love-crypto and bitmama. I don't believe any of them have an active integration. We can look into fixing the love crypto url, but usually its just the testnet env that's used here for validation when the partner was first integrating, so likely its just not active.

@dawsbot
Copy link
Contributor Author

dawsbot commented Jul 11, 2024

I'm not sure what you mean @satish-ravi - so perhaps you could edit this PR and merge away. As a new developer I find it high-risk that there's only one env that works. If zero of the provided envs were to work I would have never began contributing and assumed this was a dead project 🙏

Copy link
Member

@silasbw silasbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems low risk and simple enough to re-add a config w/ some fixes.

@silasbw silasbw enabled auto-merge (squash) July 11, 2024 14:53
@silasbw silasbw merged commit fd1dd1b into fiatconnect:main Jul 11, 2024
1 check passed
@dawsbot dawsbot deleted the fix-remove-invalid-envs branch July 12, 2024 01:47
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.

3 participants