- Notifications
You must be signed in to change notification settings - Fork928
Return template parameters in consistent order#2975
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Looks good to me. Fun change!
| // Sort variables by (filename, line) to make the ordering consistent | ||
| variables := make([]*tfconfig.Variable, 0, len(module.Variables)) | ||
| for _, v := range module.Variables { | ||
| variables = append(variables, v) | ||
| } | ||
| sort.Slice(variables, func(i, j int) bool { | ||
| return compareSourcePos(variables[i].Pos, variables[j].Pos) | ||
| }) |
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 is pretty cool. It's awesome that customers will be able to control the order that variables display in the UI 👀
This PR makes the order in which we display and prompt for template parameters deterministic and consistent. It involves two different functional changes:
ParameterSchemamessages to match the order of theirvariableblocks in the template source (first lexicographically by filename, then by line number)parameter_schemas.indexdatabase column, and sorts by that column when querying for parametersFor template versions that existed prior to this change, the database migration initializes the index column by putting the variable names for each version in lexicographic order, since we don't know the original source order.
Fixes#2471