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

fix bug with 2 templates, same ID, different paths #1573

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

eikenb
Copy link
Contributor

@eikenb eikenb commented Apr 11, 2022

The way template configs were being tracked meant that if 2 templates
had the same content but different config otherwise, they would only use
the last config. Having more than 1 template with the same content is
rare but a user ran into it with 2 templates and the same path.

This eliminates the separate tracking of template configs and bundles
them with the templates themselves. Having a 1-1 template/config and
eliminating the separate data structure makes this area of code simpler
to understand and work with while fixing this issue.

This also fixes the TemplateConfigMapping() call which should have kept
backwards compatibility with the signature.

@eikenb eikenb added the bug label Apr 11, 2022
@eikenb eikenb added this to the v0.29.0 milestone Apr 11, 2022
@eikenb eikenb requested a review from a team April 11, 2022 22:04
The way template configs were being tracked meant that if 2 templates
had the same content but different config otherwise, they would only use
the last config. Having more than 1 template with the same content is
rare but a user ran into it with 2 templates and the same path.

This eliminates the separate tracking of template configs and bundles
them with the templates themselves. Having a 1-1 template/config and
eliminating the separate data structure makes this area of code simpler
to understand and work with while fixing this issue.

This also fixes the TemplateConfigMapping() call which should have kept
backwards compatibility with the signature.
Copy link

@mkam mkam left a 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!

@eikenb eikenb merged commit 1856591 into master Apr 12, 2022
@eikenb eikenb deleted the same-template-diff-paths branch April 12, 2022 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants