Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

task: moving main.go to cmd/app/main.go #127

Merged
merged 13 commits into from
Apr 7, 2022
Merged

task: moving main.go to cmd/app/main.go #127

merged 13 commits into from
Apr 7, 2022

Conversation

paganotoni
Copy link
Member

@paganotoni paganotoni commented Mar 20, 2022

This is related to #115. It moves the generated application main to the cmd/app folder. It ensures that newly generated apps have the main.go file in there and the refresh configuration file references it.

While working on this I wondered if we should also decouple the things we are embedding within the CLI (migrations and tasks) and move these into a separate binary we add in the cmd folder, maybe cmd/cli. I've seen some people mention that its Go style to have multiple binaries for the same codebase. But this is part of another conversation.

Changes needed to complete this task:

@paganotoni paganotoni marked this pull request as ready for review April 2, 2022 02:04
@paganotoni paganotoni requested review from fasmat and sio4 April 2, 2022 14:13
@fasmat
Copy link
Member

fasmat commented Apr 2, 2022

I added just a few small comments, that should be easily fixable.

Looks good to me! Thanks for the effort @paganotoni !

@paganotoni
Copy link
Member Author

Thanks @fasmat! I don't see those comments yet. I know the Github UI for review sometimes is tricky. Can you confirm you submitted those comments? Appreciate it.

internal/genny/build/bin.go Show resolved Hide resolved
internal/genny/build/bin_test.go Show resolved Hide resolved
internal/genny/fix/refresh.go Outdated Show resolved Hide resolved
@fasmat
Copy link
Member

fasmat commented Apr 2, 2022

Sorry I hadn't submitted my review. 😅

Copy link
Member

@sio4 sio4 left a comment

Choose a reason for hiding this comment

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

It looks good to me! I tested a simple generation sequence (buffalo new, dev, build) and it worked well.

@paganotoni paganotoni merged commit 2fb4959 into main Apr 7, 2022
@paganotoni paganotoni deleted the task-moving-main branch April 7, 2022 01:16
@paganotoni paganotoni mentioned this pull request Apr 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants