Closed terencehonles closed 1 year ago
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 ?
@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.
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.