feat: added ability to pass a custom message #19
Conversation
|
Thanks for the contribution! Unfortunately we can't verify the commit author(s): David Aghassi <d***@i***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce.com Contributor License Agreement and this Pull Request will be revalidated. |
|
this is a good change, but I would prefer we use a lodash template so it can be overridden entirely instead of just appended to |
|
@jdxcode Can you provide me an example of what you mean by "lodash template"? I'm happy to update the PR once I understand 😄 EDIT: I assume you mean this? |
|
@jdxcode Ok I refactored it. I think that is what you meant. Also, this is my first jump into TypeScript, so if I messed the syntax up I apologize. |
Aghassi
force-pushed
the
feat/custom-update-message
branch
from
e4475a8 to
daca495
Compare
|
@jdxcode Does this repo support any CI for PRs? I want to make sure that my changes aren't going to break anything. I've run Also, would you be opposed to me adding husky in a separate PR so that certain checks running before commit and push locally? As an aside, can this change be squash merged? I know it is a bit messy as I figure things out. |
README.md
Outdated
| @@ -61,6 +64,7 @@ any of the following configuration properties: | |||
| ], | |||
| "warn-if-update-available": { | |||
| "timeoutInDays": 7, | |||
| "message": "Run @scope/my-cli update to update to the latest" | |||
There was a problem hiding this comment.
Running yarn lint locally passes 😞
|
@jdxcode Anything else you need on my end to get this merged in? |
|
Well I guess it wouldn’t get caught lol. Will fix.
…--
~David Aghassi
On Feb 9, 2019, 3:32 PM -0800, Kyle Brown ***@***.***>, wrote:
@krohrsb commented on this pull request.
In README.md:
> @@ -61,6 +64,7 @@ any of the following configuration properties:
],
"warn-if-update-available": {
"timeoutInDays": 7,
+ "message": "Run @scope/my-cli update to update to the latest"
It's just in the readme :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
.npmrc
Outdated
| @@ -0,0 +1 @@ | |||
| registry=https://registry.yarnpkg.com/ | |||
There was a problem hiding this comment.
Yup. I was trying to handle work from my work laptop and it was using our internal registry. I will remove this.
package.json
Outdated
| @@ -8,10 +8,12 @@ | |||
| "@oclif/command": "^1.5.3", | |||
| "@oclif/config": "^1.8.7", | |||
| "@oclif/errors": "^1.2.2", | |||
| "@types/lodash.template": "4.4.4", | |||
There was a problem hiding this comment.
this should be a dev dependency
src/hooks/init/check-update.ts
Outdated
| @@ -2,6 +2,7 @@ import {Hook} from '@oclif/config' | |||
| import Chalk from 'chalk' | |||
| import {spawn} from 'child_process' | |||
| import * as fs from 'fs-extra' | |||
| import template = require('lodash.template') | |||
There was a problem hiding this comment.
this import could be moved down lower
|
@jdxcode Addressed comments |
Currently, this warning just display line that says "You are outdated, please update". However, a user may not know what to do if this is their first time using your CLI. It may be useful to provide the user some context on how to update.
For example, we use
brewas our install method of choice. A user may assume they should runbrew upgradeto get the latest (not the case). To make it more obvious, we can hint to the user by saying "Please run my-cli update to update". This change adds that ability.