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

Comparing semver to know if we need to update #43

Merged
merged 1 commit into from
Oct 29, 2022

Conversation

shapirone
Copy link
Collaborator

Summary

We shouldn't prompt the user to upgrade if they're on a newer version. This is being put in place now that we're going to have a suggested version rather than when we always just used latest.

Testing

Change your versions manually and then run the hook locally

deno run -q --config=deno.jsonc --allow-run --allow-read --allow-write --allow-net <your-local-path>/deno-slack-hooks/src/install_update.ts

Requirements (place an x in each [ ])

@shapirone shapirone requested a review from a team as a code owner October 29, 2022 01:11
@codecov
Copy link

codecov bot commented Oct 29, 2022

Codecov Report

Merging #43 (6544551) into main (6f8fc90) will increase coverage by 0.36%.
The diff coverage is 76.47%.

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
+ Coverage   75.75%   76.11%   +0.36%     
==========================================
  Files           9        9              
  Lines         433      448      +15     
  Branches       45       47       +2     
==========================================
+ Hits          328      341      +13     
- Misses        103      105       +2     
  Partials        2        2              
Impacted Files Coverage Δ
src/install_update.ts 64.84% <0.00%> (-0.52%) ⬇️
src/check_update.ts 75.86% <86.66%> (+1.09%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@cchensh cchensh left a comment

Choose a reason for hiding this comment

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

Look good to me and tested locally, thank you for putting fix so quickly!

Test:

  1. Added "check-update": "deno run -q --config=deno.jsonc --allow-run --allow-read --allow-write --allow-net https://raw.githubusercontent.com/slackapi/deno-slack-hooks/654455189bd127d03782b0fc6ec91682c2ba0601/src/install_update.ts"
  2. Ran hermes upgrade
  3. SDKs updated
  4. Ran hermes upgrade again
  5. Seeing You are using the latest Slack CLI and SDK instead of downgrading

@cchensh cchensh merged commit 90f8f36 into main Oct 29, 2022
@cchensh cchensh deleted the neil-add-semver-comparison branch October 29, 2022 01:55
@cchensh cchensh mentioned this pull request Oct 29, 2022
2 tasks
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.

2 participants