-
Notifications
You must be signed in to change notification settings - Fork 9
feat: move existing templates-W-21163895 #840
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
base: main
Are you sure you want to change the base?
Conversation
| import { getCustomTemplates, runGenerator } from '../../utils/templateCommand.js'; | ||
| import { outputDirFlag } from '../../utils/flags.js'; | ||
| import { getCustomTemplates, runGenerator } from '../../../../utils/templateCommand.js'; | ||
| import { outputDirFlag } from '../../../../utils/flags.js'; | ||
|
|
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.
Let's rename this file (template/generate/project/generate.ts) file to be template/generate/project/index.ts instead.
This will allow use to run sf template generate project (no second generate) and it will still keep the project dir for future sub commands if we need them
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.
webapp might also need to rename to index.ts too. currently is generate.ts
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 file shows as no changes?
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.
no I just rename from site/create.nut.ts to site.nut.ts
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.
Ah, got it. My mistake 👍
| public static readonly summary = messages.getMessage('summary'); | ||
| public static readonly description = messages.getMessage('description'); | ||
| public static readonly aliases = ['force:analytics:template:create']; | ||
| public static readonly aliases = ['force:analytics:template:create', 'analytics generate template']; |
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.
For consistency, lets use : in all the aliases. Note that you can still call analytics generate template and this alias will work.
| public static readonly aliases = ['force:analytics:template:create', 'analytics generate template']; | |
| public static readonly aliases = ['force:analytics:template:create', 'analytics:generate:template']; |
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.
Check the other aliases that were added
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.
it throw an error: the Salesforce CLI will match the command words in any order, so this alias is unnecessary. when I tried to do analytics:gnerate:template ,unless I add eslint-disable-next-line. :(
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.
Oh funny. I think you just found an far edge case. Here is the linting rule: https://github.com/salesforcecli/eslint-plugin-sf-plugin/blob/19bb3dcbeab9d0427a64030e3fae4cc135810d96/src/rules/no-unnecessary-aliases.ts#L43-L44
I think this is complaining because of the every checks. The new command is:
template generate analytics template
and the alias (after splitting on a :) would be
analytics generate template
The linting rule doesn't noticed that the word template is in there twice, only that is matches
I confirmed that the alias does work locally so it's not an issue 🤷
| @@ -11,7 +11,7 @@ import { nls } from '@salesforce/templates/lib/i18n/index.js'; | |||
| import assert from 'yeoman-assert'; | |||
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.
Unrelated to anything you did: Lets rename this directory to match the command (add a -) static-resource
| "description": "Create an apex class or trigger." | ||
| }, | ||
| "digital-experience": { | ||
| "description": "Create a Digital Experience site." |
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.
I think this got removed in a merge conflict. It happened to me locally. Add this back:
"digital-experience": {
"description": "Create a Digital Experience site."
},
What does this PR do?
Moves all the exist templates to their new location.
Adds aliases aliases
Updates tests
Removes unused topics
What issues does this PR fix or reference?
@W-21163895@