-
Notifications
You must be signed in to change notification settings - Fork 0
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
(AD-1026) Bump puppeteer-lottie #2
base: master
Are you sure you want to change the base?
Conversation
@@ -40,6 +40,6 @@ | |||
}, | |||
"dependencies": { | |||
"commander": "^4.1.1", | |||
"puppeteer-lottie": "github:adpipe/puppeteer-lottie" | |||
"puppeteer-lottie": "github:adpipe/puppeteer-lottie#06487ee3c4e9aaf399443f281a7058cd7c18d192" |
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.
should we be using #master
instead of specifying a commit?
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.
honestly I was thinking about going in the opposite direction and specifying sha at both the CLI and smallville level, to facilitate easy rollback. thoughts?
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.
oooh how does it facilitate easy rollback? I thought you'd have to come in here and change the commit SHAs anyway
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 guess I mean moving forward, the next change can just be a sha change in smallville, which will pickup the correct sha in puppeteer-lottie-cli
which will pick up the correct sha in puppeteer-lottie
. then rolling back the sha specification will ensure that we're not randomly caching a version pin. If i merge this change now, the next smallville version will pick it up, because right now it's picking master.
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.
ohhh yeah i see what you're saying that makes sense
No description provided.