Skip to content

chore: unify template naming #15757

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Dec 5, 2024
Merged

chore: unify template naming #15757

merged 6 commits into from
Dec 5, 2024

Conversation

phorcys420
Copy link
Member

@phorcys420 phorcys420 commented Dec 4, 2024 •
edited
Loading

This PR changes template names and docs to follow the <provider>-<os/whatever> format for all templates.
I've decided not to split this into multiple PRs because I'd have to edit rebase the other PRs once one of them gets merged, this should be relatively low-impact anyways.

This aligns with our goals to make templates more user-friendly.

Closes #15754

@phorcys420 phorcys420 changed the title phorcys/unify template naming chore: unify template naming Dec 4, 2024
@phorcys420 phorcys420 enabled auto-merge (squash) December 4, 2024 23:06
@phorcys420 phorcys420 marked this pull request as draft December 4, 2024 23:17
auto-merge was automatically disabled December 4, 2024 23:17

Pull request was converted to draft

@phorcys420
Copy link
Member Author

I don't know why all these checks are failing, I'll take a look tomorrow.

@matifali
Copy link
Member

matifali commented Dec 5, 2024

You need to run make gen too.
Also make fmt for any possible formatting issues.

matifali
matifali approved these changes Dec 5, 2024
View reviewed changes
Copy link
Member

@matifali matifali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
It could be a breaking change if someone is initializing templates in CI.

@bpmct fir visibility.

@matifali matifali requested a review from hugodutka December 5, 2024 09:15
@matifali
Copy link
Member

matifali commented Dec 5, 2024

@phorcys420 you may also need to run make update-golden-files as we use the template ids for some CLI tests.
See: https://github.com/coder/coder/actions/runs/12176090929/job/33961115484?pr=15757#step:7:579

hugodutka
hugodutka reviewed Dec 5, 2024
View reviewed changes
Copy link
Contributor

@hugodutka hugodutka left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've started tracking usage of template examples in telemetry with the current coder release. The PR was merged last week: #15671. We use the example IDs to identify the templates in telemetry, so changing them will confuse the statistics. It's fine if we change the IDs now, but they should remain static in the future. @phorcys420, could you add the following comment in examples/examples.go above the //go:embed directives?

Template paths should not change over time. They are used as persistent identifiers in telemetry.

@phorcys420
Copy link
Member Author

@hugodutka thanks for pointing this out!
I guess we have good timing then! I wanted to do this to set a standard naming convention to avoid confusing our users so I think it will also help you if the names are standardized.

@phorcys420 phorcys420 marked this pull request as ready for review December 5, 2024 16:52
@phorcys420
Copy link
Member Author

@matifali is there a way we can merge without weekly-docs passing?

@phorcys420 phorcys420 closed this Dec 5, 2024
@phorcys420 phorcys420 reopened this Dec 5, 2024
@phorcys420
Copy link
Member Author

whoops

@matifali matifali merged commit 069655a into main Dec 5, 2024
62 of 66 checks passed
@matifali matifali deleted the phorcys/unify-template-naming branch December 5, 2024 17:37
@phorcys420 phorcys420 mentioned this pull request Dec 5, 2024
Merged
phorcys420 added a commit that referenced this pull request Dec 6, 2024
@phorcys420
chore: add template ID disclaimer (#15770)
aa310e3 
Follow-up to #15757
aslilac pushed a commit that referenced this pull request Dec 11, 2024
@phorcys420 @aslilac
chore: add template ID disclaimer (#15770)
ae57f58 
Follow-up to #15757
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugodutka hugodutka hugodutka left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@matifali matifali matifali approved these changes

@bpmct bpmct Awaiting requested review from bpmct

@johnstcn johnstcn Awaiting requested review from johnstcn

@EdwardAngert EdwardAngert Awaiting requested review from EdwardAngert

Assignees

@phorcys420 phorcys420

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unify template naming
3 participants
@phorcys420 @matifali @hugodutka