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

chore: add version command #310

Merged
merged 1 commit into from
Feb 24, 2023
Merged

Conversation

silasb
Copy link
Contributor

@silasb silasb commented Feb 22, 2023

What are you trying to accomplish?

I was debugging issues with the newest 2.3.0 release and was having difficulties knowing what version I was running.

I noticed an error (I can pull this out into another PR if needed) while running rake, it failed to run specs because a missing require "pathname".

What approach did you choose and why?

I choose to add a version command in to make it easy for anyone running bundle exec packwerk to get the version. It feels like best practice to always provide a version command or flag for a CLI application.

What should reviewers focus on?

The output. I'm not sure if we'd want to use 'v' prefix in-front of the version number.

Type of Change

  • Bugfix
  • New feature
  • Non-breaking change (a change that doesn't alter functionality - i.e., code refactor, configs, etc.)

Additional Release Notes

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • It is safe to rollback this change.

@silasb silasb requested a review from a team as a code owner February 22, 2023 23:19
Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

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

LGTM, but this made me realize we have a dead autoload entry here. Can you remove it and squash your commits? Thanks

@@ -3,6 +3,8 @@

ENV["RAILS_ENV"] = "test"

require "pathname"
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed for our setup, but I assume you ran tests without rake to get the error (via ruby -I). In that case, we can keep it 👍

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 an issue on my part, I ran with global rake and saw this issue, running with bundle exec rake resolved it.

* Added in test for version command
* Removed dead auto autoloader for Version
@gmcgibbon gmcgibbon merged commit 868fa97 into Shopify:main Feb 24, 2023
@gmcgibbon
Copy link
Member

Thanks! For anyone else having issues with this on version of packwerk, you can use bundle info packwerk in the interim.

gmcgibbon added a commit that referenced this pull request Mar 1, 2023
The version isn't required, so the version command won't work when the
gemspec isn't loaded (eg. when used in an app). #310
@gmcgibbon gmcgibbon mentioned this pull request Mar 1, 2023
7 tasks
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems March 1, 2023 19:58 Inactive
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