-
Notifications
You must be signed in to change notification settings - Fork 367
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
Cli refactor #376
base: main
Are you sure you want to change the base?
Cli refactor #376
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should meet and discuss
@@ -0,0 +1,54 @@ | |||
name: Linting Generated Blueprints Advanced |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the frontend linter need to know the backend matrix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend framework, DB drivers and Git are required inputs for project creation. And we are looping over all backends to ensure we don't have templating issues on any of them when they are paired with frontend.
|
||
Note: Selecting Tailwind option will automatically select HTMX unless React is explicitly selected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we should still keep this here to let people know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In new implementation this is not an issue anymore. In frontend mode you choose framework and then there is a prompt for frontend-advanced options, in this case Tailwind
@@ -152,67 +181,56 @@ Blueprint UI is a web application that allows you to create commands for the CLI | |||
Here's an example of setting up a project with a specific database driver: | |||
|
|||
```bash | |||
go-blueprint create --name my-project --framework gin --driver postgres --git commit | |||
go-blueprint create -n my-project -b gin -d postgres -g commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For discussion:
I dont know if i like the move to change the backend go frameworks to backend. They are still frameworks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long flag can be --backend-framework, I will push the changes. And then we can discuss what else can be improved or removed :)
|
||
Websocket: | ||
Advanced features are accessible with the `-a` flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag redesign makes sense in certain areas, but something this major definitely needs to be fully discussed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other flags are not redefined, only the -framework is now -b or --backend-framework. All flags had a short and long notation, we always used the long one. For example --advanced, which also had the short -a. It is unusual for primary flags to be used in long notation in examples, and can be confusing with secondary flags that have only the long input (--feature, --frontend-framework or --frontend-advanced).
blueprint-ui/Dockerfile
Outdated
RUN go install github.com/a-h/templ/cmd/templ@latest && \ | ||
templ generate && \ | ||
curl -sL https://github.com/tailwindlabs/tailwindcss/releases/download/v3.4.10/tailwindcss-linux-x64 -o tailwindcss && \ | ||
chmod +x tailwindcss && \ | ||
./tailwindcss -i cmd/web/styles/input.css -o cmd/web/assets/css/output.css |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dockerfile is only for respective frontend flags triggered?
How come there is tailwind + templ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is from blueprint-ui, and I moved code to a separate branch and PR so that it is less confusing. #377
blueprint-ui/Makefile
Outdated
tailwind-install: | ||
@if [ ! -f tailwindcss ]; then curl -sL https://github.com/tailwindlabs/tailwindcss/releases/download/v3.4.10/tailwindcss-linux-x64 -o tailwindcss; fi | ||
|
||
@chmod +x tailwindcss | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think this needs to be a make file for the users to use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also part of bp-ui (#377) , crucial for local dev and test. Easier deployment and maintenance benefits, if new PR changes directory and file layout this part of code also needs to be updated, along with docs code. I added this requirement to the PR checklist.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.
Problem/Feature
Fronted flag implementation
Check README for flag usage
Description of Changes:
close #360
close #331
Checklist