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

adding DB support #22

Merged
merged 47 commits into from
Nov 16, 2023
Merged

Conversation

MohammadAlhallaq
Copy link
Contributor

@MohammadAlhallaq MohammadAlhallaq commented Nov 3, 2023

#11

@MohammadAlhallaq MohammadAlhallaq marked this pull request as ready for review November 4, 2023 14:55
@MohammadAlhallaq
Copy link
Contributor Author

Hey @Melkeydev,

would appreciate your feedback on the implementation, let me know what could have been done better or if there's anything I might have missed in it.

.gitignore Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cmd/template/DBDriver/mongo.go Outdated Show resolved Hide resolved
cmd/template/DBDriver/mysql.go Outdated Show resolved Hide resolved
cmd/template/DBDriver/mysql.go Outdated Show resolved Hide resolved
cmd/template/DBDriver/postgres.go Outdated Show resolved Hide resolved
@newtoallofthis123
Copy link

I'm planning on improving upon this by adding some prompts for a .env file? I mean that is I mostly use DB drivers....so would that be cool?

@newtoallofthis123
Copy link

I could also probably add some fields like

DB_URL=postgres://
DB_NAME=name

I could use the popular https://github.com/joho/godotenv package?

@Melkeydev
Copy link
Owner

I could also probably add some fields like

DB_URL=postgres://
DB_NAME=name

I could use the popular https://github.com/joho/godotenv package?

Do we need to introduce another package to lead .env vars or can we just use the standard os lib?
Either way, would it make sense to make it into a separate PR?
I am planning on reviewing just this PR and how the DB integration is set up tomorrow

@newtoallofthis123
Copy link

True....we can probably just read of the os lib as a default, but have an option of a .env? moreover, godotenv is quite a popular package with good reputation

@Melkeydev
Copy link
Owner

True....we can probably just read of the os lib as a default, but have an option of a .env? moreover, godotenv is quite a popular package with good reputation

Good points I am OK to add godotenv to either this repo or a following one

@Melkeydev
Copy link
Owner

@MohammadAlhallaq There are some new merge conflicts due to previous merges!

cmd/program/program.go Outdated Show resolved Hide resolved
Copy link
Owner

@Melkeydev Melkeydev left a comment

Choose a reason for hiding this comment

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

Added a few comments - tested out the PR everything seems to work however,
the DB doesnt actually get hooked up.
Wondering if there is a way we can fully test out this feature and implement it for a user to use

cmd/steps/steps.go Outdated Show resolved Hide resolved
@MohammadAlhallaq
Copy link
Contributor Author

Added a few comments - tested out the PR everything seems to work however, the DB doesnt actually get hooked up. Wondering if there is a way we can fully test out this feature and implement it for a user to use

I will tackle those asap, and try to figure out a way to make the service accessible via a route for testing

@Melkeydev
Copy link
Owner

Added a few comments - tested out the PR everything seems to work however, the DB doesnt actually get hooked up. Wondering if there is a way we can fully test out this feature and implement it for a user to use

I will tackle those asap, and try to figure out a way to make the service accessible via a route for testing

Ping me when this is ready for a another review :)

cmd/program/program.go Outdated Show resolved Hide resolved
.idea/go-blueprint.iml Outdated Show resolved Hide resolved
Copy link

@BKR-dev BKR-dev left a comment

Choose a reason for hiding this comment

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

LGTB

@Melkeydev Melkeydev changed the base branch from main to staging November 16, 2023 06:17
@Melkeydev Melkeydev changed the base branch from staging to main November 16, 2023 21:13
Copy link
Owner

@Melkeydev Melkeydev left a comment

Choose a reason for hiding this comment

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

Looks
Fucking
Incredible
To Me
My honor to MERGE THIS

@Melkeydev Melkeydev merged commit 5cfe8e3 into Melkeydev:main Nov 16, 2023
37 checks passed
@MohammadAlhallaq
Copy link
Contributor Author

Looks Fucking Incredible To Me My honor to MERGE THIS

thanks man, I've really enjoyed working on it.

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.

None yet

7 participants