go-fuego / fuego

Golang Fuego - web framework generating OpenAPI 3 spec from source code
https://go-fuego.github.io/fuego/
MIT License
918 stars 46 forks source link

Remove Group Parameters #203

Closed EwenQuim closed 1 day ago

EwenQuim commented 1 month ago

This PR aims to remove Group Parameters in favor of reusable Route Options. While it seems to be a regression, it is more in the spririt of Go:

@dylanhitt could you please review it?

dylanhitt commented 1 month ago

Not sure I really agree with this.

Being able to declare a common header at the first Server level seems to be very useful, no? This also could cause quite a bit of duplicate code for our callers depending on how they structure their projects. So for instance it's not uncommon for people to separate route grouping into separate packages. This then either requires the caller to duplicate that option definition var into all of these packages or create a separate package just to hold their own option funcs, which it isn't really go like to have a util package.

I will review this as it is, but I'm not really understanding the benefit of removing this. The code simplification doesn't really seem to high, and as I pointed out above route options are more composable, but only when dealing with a single package. In large projects it's inevitable that routes will be separated into different pkgs just for logical simplicity.

If the plan is to eventually add the options style configuration on to the Server/Group. I'd be 100% on board.

Just my two cents

Cheers

EwenQuim commented 1 month ago

If the plan is to eventually add the options style configuration on to the Server/Group. I'd be 100% on board.

Yes I hesitated, we might let the final decision to the users.

We might do this and keep the same signature as the route options, which will not be trivial.

EwenQuim commented 1 day ago

Closed in favor of #233