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

Add generated samples of each template #65

Merged
merged 4 commits into from
Aug 4, 2024
Merged

Add generated samples of each template #65

merged 4 commits into from
Aug 4, 2024

Conversation

joshka
Copy link
Member

@joshka joshka commented Jul 14, 2024

No description provided.

Copy link
Sponsor Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

LGTM, but how are we planning to keep these up-to-date? One thing that I can think of is that checking the diff in the CI and failing if the generated ones are not updated. Or, even better, we can add a pre-commit hook.

What do you think?

justfile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@joshka
Copy link
Member Author

joshka commented Jul 26, 2024

LGTM, but how are we planning to keep these up-to-date? One thing that I can think of is that checking the diff in the CI and failing if the generated ones are not updated. Or, even better, we can add a pre-commit hook.

I'm fine with manually handling this until it's really a pain. I'd much prefer not to have pre-commit hooks doing this update (even though it's actually fairly fast), as these make destructive changes to your workspace.

In general, making changes to a template should look like:

  1. change the template
  2. re-generate the template using just generate-...
  3. check changes in generated files look ok, add to git, but don't commit yet
  4. run clippy and fmt to check if these make any changes (if so go back to 1...)
  5. commit and PR

Ideally the generated files should not have clippy / formatting problems, because this just pushes these problems out to every end-user that uses these, so fixing them once is a good use of our time.

Copy link
Member Author

@joshka joshka left a comment

Choose a reason for hiding this comment

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

I added a workspace file to make it easier to run cargo check / clippy / fmt at the repo root

README.md Show resolved Hide resolved
@joshka
Copy link
Member Author

joshka commented Aug 2, 2024

I need to fix the failing check on this at some point

@orhun
Copy link
Sponsor Member

orhun commented Aug 2, 2024

CI logs go crazy for some reason, I guess it's an issue with cargo generate's prompt..

@joshka
Copy link
Member Author

joshka commented Aug 2, 2024

CI logs go crazy for some reason, I guess it's an issue with cargo generate's prompt..

It's due to the custom code that checks for params, but I removed some of them.

@orhun
Copy link
Sponsor Member

orhun commented Aug 4, 2024

Thanks for the pointer @joshka, should be fixed in 73225ec

@orhun orhun merged commit 0ab4e27 into main Aug 4, 2024
3 checks passed
@orhun orhun deleted the jm/generated branch August 4, 2024 08:29
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