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

adds version option #68

Merged
merged 6 commits into from
Feb 22, 2023
Merged

adds version option #68

merged 6 commits into from
Feb 22, 2023

Conversation

LucaSforza
Copy link
Contributor

As requested in #66 I created the option to print the version on dbtc

@dpguthrie
Copy link
Owner

Thanks for opening this up @LucaSforza, I appreciate the contribution! Couple of comments:

  • I think __version__ should exist in its own file - _version.py
  • I'd like to investigate using this approach here to try and have a single place where version needs to be updated with poetry version <version>

@LucaSforza
Copy link
Contributor Author

I added the function version(__package__) imported from module importlib_metadata,so I no longer use any global variables.

Now I should have solved the problem, tell me if everything is OK

@dpguthrie
Copy link
Owner

Thanks for making some of those changes. Couple things that I'd like to see though:

  • I think the __version__ variable should be in its own _verison.py file next to __init__.py.
  • This variable should be imported into __init__.py also so it's available to the python package as well.

Also, it looks like your last commit included some irrelevant files .DSStore.

@LucaSforza
Copy link
Contributor Author

Ok i created the file _version.py with inside has the variable __version__ ,please update me if everything is fine this time

@dpguthrie
Copy link
Owner

Could you remove the .DSStore file in this PR?

@LucaSforza
Copy link
Contributor Author

Ok, sorry I didn't see that there were others left, now they should all have been deleted

@dpguthrie dpguthrie merged commit ba4c2f0 into dpguthrie:main Feb 22, 2023
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