jan-leila/vegan-barcode
1
0
Fork 0
Code Issues Pull requests 1 Actions Packages Projects Releases Wiki Activity

tb/main #1

Open
terrabytten wants to merge 17 commits from tb/main into main
pull from: tb/main
merge into: jan-leila:main
Conversation 0 Commits 17 Files changed 19 +814 -155
terrabytten commented 2025-04-17 23:04:41 +00:00
Collaborator
Copy link

This pull request connects all sections of the application and resolves issues with internal package imports. Naming is not finalized and still slightly messy, but the test function can successfully add an entry to the database.

Additionally, documentation is added on deleting and creating the database in the readme.

This pull request connects all sections of the application and resolves issues with internal package imports. Naming is not finalized and still slightly messy, but the test function can successfully add an entry to the database. Additionally, documentation is added on deleting and creating the database in the readme.
terrabytten added 6 commits 2025-04-17 23:04:42 +00:00
terrabytten requested review from jan-leila 2025-04-17 23:04:54 +00:00
jan-leila reviewed 2025-04-17 23:17:50 +00:00
internal/api/routes.go Outdated
@ -0,0 +44,4 @@
router.Run("localhost:8080")
}
func (s *ApiService) runTest(c *gin.Context) {
jan-leila commented 2025-04-17 23:11:45 +00:00
Owner
Copy link

what is the goal of this api folder, is it to:
a. bind API routes to functions
or b. parse out the parameters of our queries to then pass to the called service?

I think it would be fine to have both of these in the same folder but it might get to be a bit messy to have both of them in one place. Ideally a would live somewhere related to our swagger.yml file but that's just in the directory above us right now. Maybe we can move the swagger file into this folder, then take runTest (or its future non test equivalents) and create a new folder to host them. For Kotlin projects that I have worked on generally gets a controllers folder but I'm open to other names if you can thing of something that makes more sense to you

what is the goal of this `api` folder, is it to: a. bind API routes to functions or b. parse out the parameters of our queries to then pass to the called service? I think it would be fine to have both of these in the same folder but it might get to be a bit messy to have both of them in one place. Ideally a would live somewhere related to our `swagger.yml` file but that's just in the directory above us right now. Maybe we can move the swagger file into this folder, then take `runTest` (or its future non test equivalents) and create a new folder to host them. For Kotlin projects that I have worked on generally gets a `controllers` folder but I'm open to other names if you can thing of something that makes more sense to you
internal/api/routes.go Outdated
@ -0,0 +48,4 @@
queryParam := c.Param("id")
log.Debug("Test was successful.")
err := s.db.Insert(c, database.ProductsTable, &database.Product{System: "upc", Barcode: "fubar", Created_at: time.Now()})
jan-leila commented 2025-04-17 23:14:12 +00:00
Owner
Copy link

We shouldnt really be making DB calls here generally best practices would be to mak some function in our database folder that for example is called createProduct that we would just call with our System, and Barcode value. Then we would also want to have that database::createProduct call done within our services folder (to keep a separation of our network layer and our business logic) but that ones not as big of a deal because this is just a test function that doesnt really have a defined goal in mind

We shouldnt really be making DB calls here generally best practices would be to mak some function in our database folder that for example is called `createProduct` that we would just call with our `System`, and `Barcode` value. Then we would also want to have that `database::createProduct` call done within our `services` folder (to keep a separation of our network layer and our business logic) but that ones not as big of a deal because this is just a test function that doesnt really have a defined goal in mind
terrabytten marked this conversation as resolved
internal/api/routes.go Outdated
@ -0,0 +51,4 @@
err := s.db.Insert(c, database.ProductsTable, &database.Product{System: "upc", Barcode: "fubar", Created_at: time.Now()})
if err != nil {
// TODO: Figure out correct status code.
c.JSON(http.StatusInternalServerError, gin.H{"message": "Failed to insert item to product table", "error": err.Error()})
jan-leila commented 2025-04-17 23:16:20 +00:00
Owner
Copy link

c.JSON I believe pipes out the passed data as a stringify value on whatever network socket the gin context has but doesnt close that connection so when we get an error we end up getting something roughly like error: xyz... success xyz... we want to close that connection and return early before we get to the success portion of this method

c.JSON I believe pipes out the passed data as a stringify value on whatever network socket the gin context has but doesnt close that connection so when we get an error we end up getting something roughly like `error: xyz... success xyz...` we want to close that connection and return early before we get to the success portion of this method
terrabytten added 1 commit 2025-04-21 23:30:17 +00:00
jan-leila added 1 commit 2025-04-21 23:54:53 +00:00
jan-leila added 1 commit 2025-04-22 00:10:38 +00:00
terrabytten added 2 commits 2025-04-22 00:17:50 +00:00
jan-leila added 1 commit 2025-04-22 00:43:19 +00:00
terrabytten added 1 commit 2025-04-22 01:43:14 +00:00
terrabytten added 1 commit 2025-04-22 01:53:37 +00:00
terrabytten added 1 commit 2025-04-23 20:34:27 +00:00
terrabytten added 1 commit 2025-04-23 21:24:06 +00:00
terrabytten added 1 commit 2025-04-23 21:29:11 +00:00
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin tb/main:tb/main
git checkout tb/main

Merge

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff tb/main
git checkout main
git merge --ff-only tb/main
git checkout tb/main
git rebase main
git checkout main
git merge --no-ff tb/main
git checkout main
git merge --squash tb/main
git checkout main
git merge --ff-only tb/main
git checkout main
git merge tb/main
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
jan-leila
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: jan-leila/vegan-barcode#1
No description provided.
Delete branch "tb/main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?