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

keep path parameters in their given order #841

Merged

Conversation

terencehonles
Copy link
Contributor

This change keeps path parameters in their given order as tools like https://github.com/swagger-api/swagger-codegen/ use the parameter ordering as is to create a function signature. Arguably the tools could re-parse the path looking for template variables and use that ordering but it appears they do not.

The swagger spec does not seem like it documents how these parameters should be ordered, and while when reading the schema document, alphabetically ordered might make sense for a human to verify the parameters exist, it's losing information about the ordering which may be conveying additional information.

@JoelLefkowitz JoelLefkowitz merged commit 5530022 into axnsan12:1.21.x Mar 8, 2023
@terencehonles terencehonles deleted the keep-path-parameter-ordering branch May 3, 2023 22:23
@riddheshSajwan
Copy link

Hi @terencehonles , @JoelLefkowitz
This PR messes up any function that was called using positional arguments instead of keyword arguments as it is completely changing the order of arguments.

it's losing information about the ordering which may be conveying additional information.

what additional information might be getting conveyed by the order of params ?

@terencehonles
Copy link
Contributor Author

terencehonles commented Aug 6, 2024

@riddheshSajwan this change should fix that issue not create it. Before the parameters were sorted, but after removing the sorting they will be in the order that they are supplied in the path, which is the order that tools like Django will call a view with.

Using tools like https://github.com/swagger-api/swagger-codegen relied on this order to create API clients which now had the incorrect order. Is this maybe what you are suggesting (that a generated client's order is now changed)?

what additional information might be getting conveyed by the order of params ?

imagine a view: /api/resource/:id/sub-resource/:start-date/:end-date/ that would be a Django view my_view(resource, start_date, end_date). If the order of the parameters were now client_method(end_date, resource, start_date) that would be a bit confusing, but that's ordering the parameters alphabetically when you'd expect to have the client client_method(resource_id, start_date, end_date) as the view would be called.

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.

3 participants