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

feat: separate plugin-templates from this repository #520

Merged
merged 9 commits into from
Nov 14, 2022

Conversation

randi274
Copy link
Contributor

What does this PR do?

Removes the plugin-templates library from this repository. Plugin-templates will now be a stand-alone repository here.

What issues does this PR fix or reference?

@W-11309541@

Changed the monorepo to a single repo. The plugin has been moved to
https://github.com/salesforcecli/plugin-templates.

BREAKING CHANGE: The plugin is no longer available here.
@randi274 randi274 requested a review from a team as a code owner October 31, 2022 20:34
@@ -41,6 +41,7 @@
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"commitizen": "^4.2.4",
"cz-conventional-changelog": "^3.3.0",

Choose a reason for hiding this comment

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

Is this something I should also add to the apex-node library?

},
"engines": {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably put engines back.

@@ -11,6 +11,7 @@
"outDir": "./lib",
"preserveConstEnums": true,
"strict": true,
"allowSyntheticDefaultImports": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

did something change resulting in this addition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was present in the original templates-level file that I had moved to the top level, so I think the diff view is a little confused: https://github.com/forcedotcom/salesforcedx-templates/blob/8cb2f51d25a054db61891c52f5ad24802677ce1c/packages/templates/tsconfig.json

.nycrc Outdated
"nyc": {
"extends": "@salesforce/dev-config/nyc"
Copy link
Contributor

Choose a reason for hiding this comment

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

does this do that ☝️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was also present at the templates-level initially! I think Willie had added it in a previous PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And to actually answer the question ... no. https://github.com/forcedotcom/dev-config/blob/main/nyc.json

I'm thinking I might want to revert in that case?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah agreed, probably want to go with what we had originally

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

Ran though the PR as best I could and had a couple of trivial suggestions. Pulled local and verified all the build pieces. Only issue is lint isn't working currently

question Which command would you like to run?: lint
$ eslint -c .eslintrc.json --ext .ts ./src ./test

Oops! Something went wrong! :(

ESLint: 7.32.0

ESLint couldn't find the config "../.eslintrc.js" to extend from. Please check that the name of the config is correct.

The config "../.eslintrc.js" was referenced from the config file in "/Users/gbockus/github/PDT/salesforcedx-templates/test/.eslintrc.js".

If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.

error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ansible|gbockus-ltm at ~/github/PDT/salesforcedx-templates ±(randi/remove-plugin) ❯```

I think we get lint fixed and we're good to go 👍 

"node": ">=14.17.0"
"resolutions": {
"yeoman-generator": "^5.6.1",
"@types/yeoman-generator": "^5.2.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

this may no longer be necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would be super nervous to remove yeoman in here... do you think it's not necessary because it was used in plugin-templates mostly before?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is just the resolutions block. It basically ties us & the deps to a version of yeoman. It's fine we can leave it.

@randi274
Copy link
Contributor Author

randi274 commented Nov 3, 2022

@gbockus-sf linting error was fixed - also I don't think linting was running automatically and I got a ton of errors when I ran the command, so I fixed those too. I'll look into seeing if we can get it to run on commits like we do with vs code.

package.json Outdated
"./{src,test}/**/*.{ts,js}": [
"eslint -c .eslintrc.json --fix"
]
"husky": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok so adding this was pretty cool. We now get the following when we commit, and then a full lint + test happened on push!
Screen Shot 2022-11-03 at 4 59 22 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

🏆

"text",
"lcov"
],
"reporter": ["text", "lcov"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is part of the commit hook now, so it stays this way

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

🚢

@randi274 randi274 merged commit 2722a2d into main Nov 14, 2022
@randi274 randi274 deleted the randi/remove-plugin branch November 14, 2022 19:54
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