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.

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

(#2377) Add ability to configure default template name #2390

Merged
merged 4 commits into from
Dec 23, 2021
Merged

(#2377) Add ability to configure default template name #2390

merged 4 commits into from
Dec 23, 2021

Conversation

gep13
Copy link
Member

@gep13 gep13 commented Oct 3, 2021 •
edited
Loading

Description Of Changes

A new configuration value has been added to the chocolatey.config file, name defaultTemplateName. This allows the user to specify the name of a template which should be used by default when executing the choco new command.

Motivation and Context

While it is currently possible to override the files that are used by default when running choco new, this requires the user adding the files into a known location. By adding this configuration option, it allows the user to run something like:

choco install zip.template
choco config set --name=defaultTemplateName --value=zip

And then, going forward, anytime that choco new bob is run, and new package called bob will be created using the zip template.

If the user were to run something like the following:

choco new bob --template-name=msi

Then the msi template would be used, and the defaultTemplateName would be ignored.

If the user were to override the default template in the required folder, then it will be used when nothing else is set.

However, if the user were to override the default template files in the required folder, and set the defaultTemplateName configuration option, then the latter would win.

Testing

The following things have been done to test this:

  1. choco install zip.template
  2. choco install msi.template
  3. Run choco new bob
  4. A new package with the name bob is created, using the standard template
  5. Delete the newly created bob folder
  6. Run choco new bob --template-name=zip
  7. A new package with the name bob is created, using the zip template
  8. Delete the newly created bob folder
  9. Run choco config set --name=defaultTemplateName --value=msi
  10. Run choco new bob
  11. A new package with the name bob is created, using the msi template
  12. Delete the newly created bob folder
  13. Run choco new bob --template-name=zip
  14. A new package with the name bob is created, using the zip template
  15. Delete the newly created bob folder
  16. Run choco config unset --name=defaultTemplateName
  17. Copy the files from C:\ProgramData\chocolatey\templates\msi to C:\ProgramData\chocolatey\templates\default
  18. Run choco new bob
  19. A new package with the name bob is created, using the msi template
  20. Delete the newly created bob folder
  21. Run choco new bob --template-name=zip
  22. A new package with the name bob is created, using the zip template
  23. Delete the newly created bob folder
  24. Run choco config set --name=defaultTemplateName --value=zip
  25. Run choco new bob
  26. A new package with name bob is created, using the zip template

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Fixes #2377

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated - added to tracking issue here: Documentation updates for v0.12.0 #2391
  • Tests to cover my changes, have been added
  • All new and existing tests passed.

@gep13 gep13 requested review from ferventcoder and AdmiringWorm and removed request for ferventcoder October 3, 2021 19:36
@gep13
Copy link
Member Author

gep13 commented Oct 3, 2021

@AdmiringWorm did you have any suggestions about additional Unit Tests that should be added with this PR?

@gep13
Copy link
Member Author

gep13 commented Oct 3, 2021

@TheCakeIsNaOH would be great if you could take a look at this PR as well, and to get your thoughts on it.

@AdmiringWorm
Copy link
Member

@AdmiringWorm did you have any suggestions about additional Unit Tests that should be added with this PR?

From looking at the changed code, there are two unit tests that at least should be added.

  1. Test that the configured default template value is being used.
  2. That the passed in template name (configuration.NewCommand.TemplateName) is respected.

Probably not needed to do a full run for each of those two tests, but would need to assert that the file service is called with the correct values.

There are already a few tests for the template service which can be built upon to test those two: https://github.com/chocolatey/choco/blob/develop/src/chocolatey.tests/infrastructure.app/services/TemplateServiceSpecs.cs

@TheCakeIsNaOH
Copy link
Member

I like the behavior, and the code looks good to me.

@gep13 gep13 marked this pull request as ready for review October 11, 2021 20:14
@gep13
Copy link
Member Author

gep13 commented Oct 11, 2021

@AdmiringWorm @TheCakeIsNaOH I have moved this away from draft, would both of you be able to take a look, and let me know if you have any comments. Thanks

@gep13 gep13 force-pushed the feature/2377-default-template-name branch from cf27684 to 8159a98 Compare October 11, 2021 20:21
@TheCakeIsNaOH
Copy link
Member

TheCakeIsNaOH commented Oct 12, 2021 •
edited
Loading

Is it just me, or is --built-in not working with if defaultTemplateName is set?

IMHO, using --build-in should override defaultTemplateName.
Here is the precedence I think should be used, but it is up for discussion:

  1. choco new --template=name
  2. choco new --built-in
  3. choco config defaultTemplateName
  4. Manually created files in $env:chocolateyinstall\templates\default
  5. Fall back onto the built-in template.

@gep13 gep13 marked this pull request as draft October 14, 2021 09:11
@TheCakeIsNaOH TheCakeIsNaOH mentioned this pull request Dec 11, 2021
Merged
8 tasks
@gep13 gep13 force-pushed the feature/2377-default-template-name branch from 8159a98 to 4ecbd3e Compare December 23, 2021 09:51
@gep13
(maint) Remove unnecessary whitespace
de61b1d
@gep13
(chocolatey#2377) Add new config value DefaultTemplateName
dafae0f 
This will be used to allow the user to control which template should be
used by default. While it is possible to allow the user to override the
default template files, this gives them a little bit more control, to
specify via configuration which template they want to use by default.
@gep13 gep13 force-pushed the feature/2377-default-template-name branch from 4ecbd3e to 7cfc82f Compare December 23, 2021 10:22
@gep13
(chocolatey#2377) Use defaultTemplateName if set
defb7d3 
If the defaultTemplateName configuration value is set, and the user
hasn't specified a template name at the command line, check to see
whether a template exists with that name. If it does, use this as the
template when generating the new package.

Given that the default value for the new defaultTemplateName
configuration value is an empty string, if the user is directly
overriding the default template files folder, this overriding will still
take place.

Also take into consideration the usage of the --built-in command line
option. If this is set, make sure to respect it. The order of
precedence is the following:

1. choco new --template=name
2. choco new --built-in
3. choco config defaultTemplateName
4. Manually created files in $env:chocolateyinstall\templates\default
5. Fall back onto the built-in template.
@gep13
(chocolatey#2377) Add unit tests
fd5b8e9 
Unit tests have been added to exercise the generate method of the
TemplateService to ensure that the template name that is used matches
the configured value.

- If option is set at the command line, it should win, even if there is
a default value in the chocolatey.config file.
- If default value is set in chocolatey.config file but path doesn't
exist, should use null value
- If default value is set in chocolatey.config file and path exists,
should be set to this template value
- If --built-in option is set, this should override default value in
chocolatey.config file
@gep13 gep13 force-pushed the feature/2377-default-template-name branch from 0c0562d to fd5b8e9 Compare December 23, 2021 10:27
@gep13 gep13 marked this pull request as ready for review December 23, 2021 10:27
@gep13
Copy link
Member Author

gep13 commented Dec 23, 2021

@TheCakeIsNaOH thank you for the initial review of this. I have fixed up the suggestion you made and used the order of precedence that you suggested, as this makes sense to me.

Going to move forward with getting this merged, and onto some other PR's.

@gep13 gep13 merged commit c76012d into chocolatey:develop Dec 23, 2021
@gep13 gep13 deleted the feature/2377-default-template-name branch December 23, 2021 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheCakeIsNaOH TheCakeIsNaOH TheCakeIsNaOH left review comments

@AdmiringWorm AdmiringWorm Awaiting requested review from AdmiringWorm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add configuration option for default template name to be used in conjunction with the choco new command
3 participants
@gep13 @AdmiringWorm @TheCakeIsNaOH