Skip to content

fix: version defaulting to 0.0.0.dev0#40

Merged
WilliamBergamin merged 8 commits intomainfrom
bill-fix-version-compare
Apr 17, 2024
Merged

fix: version defaulting to 0.0.0.dev0#40
WilliamBergamin merged 8 commits intomainfrom
bill-fix-version-compare

Conversation

@WilliamBergamin
Copy link
Copy Markdown
Contributor

Summary

This PR aims to fix #37, it appear the value being used to look up the version of the package on the users machine was defaulting to 0.0.0.dev0 making the check_update command unusable. These changes revert some of the automation introduced in #36 to fix this bug.

Testing

  1. pull the changes locally
  2. build the package
  3. test the package on an app with slack create -t https://github.com/slack-samples/bolt-python-custom-function-template

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_and_run_tests.sh after making the changes.

@WilliamBergamin WilliamBergamin added the bug Something isn't working label Apr 16, 2024
@WilliamBergamin WilliamBergamin self-assigned this Apr 16, 2024
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.55%. Comparing base (120b296) to head (9a368a3).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #40      +/-   ##
==========================================
- Coverage   94.58%   94.55%   -0.03%     
==========================================
  Files          11       11              
  Lines         203      202       -1     
==========================================
- Hits          192      191       -1     
  Misses         11       11              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

Oh fascinating catch but a bummer that the automation magic goes away for now... Totally missed that this would fallback to the dev version by default!

will live (create it if it does not exist)
- `git checkout -b future-release`
- `git commit -m 'version 1.2.3.dev0'`
- `git push future-release`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Always fascinating to see the ways to push upstream haha! No change needed, I'm just a big fan of

$ git push -u origin future-release

Comment on lines -25 to -26
env:
SLACK_CLI_HOOKS_VERSION: ${{ github.event.release.name }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🫖 Pouring one out

Copy link
Copy Markdown
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Some minor docs comments left but agreed with Ethan's point re: major downgrades, good test to add.

[run the tests](#run-all-the-unit-tests).

1. Create a new GitHub Release from the
Releases for this library are automatically generated off of git releases.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

git has no concept of releases, only tags. Did you mean GitHub Releases here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@filmaj good catch 💯 currently the Github Action that deploys a new version of the project to Pypi is triggered by publishing a new Github Release, the instructions ask to create a tag along with the GitHub release
Should this action be triggered by the tag creation instead?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think Release creation is a nice underlying trigger, as it forces the maintainer to create a GitHub Release (which in turn will create a tag).

Releases for this library are automatically generated off of git releases. Before
creating a new release, ensure that everything on the `main` branch since the
last tag is in a releasable state! At a minimum,
Releases for this library are automatically generated off of git releases.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same thing here, git tags or GitHub Releases?

@WilliamBergamin WilliamBergamin requested a review from filmaj April 17, 2024 16:14
@WilliamBergamin WilliamBergamin merged commit a13956d into main Apr 17, 2024
@WilliamBergamin WilliamBergamin deleted the bill-fix-version-compare branch April 17, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] check update seems to be wrong

5 participants