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
DM-35047: Add upload to pypi #126
Conversation
Codecov Report
@@ Coverage Diff @@
## main #126 +/- ##
=======================================
Coverage 92.08% 92.08%
=======================================
Files 42 42
Lines 2665 2665
=======================================
Hits 2454 2454
Misses 211 211 Continue to review full report at Codecov.
|
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.
Looks okay. One actual bug in the cfg file plus some questions.
- name: Set up Python | ||
uses: actions/setup-python@v3 | ||
with: | ||
python-version: 3.8 |
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.
Does this python version matter for anything? Should it match build_and_test or is it irrelevant because the code in the package is never run?
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.
No. We can use python 3.10 here
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.
Can this part of the workflow be a reusable component? It seems like there isn't anything specific to utils here.
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.
Yes. I would make this an additional ticket.
LICENSE
Outdated
|
||
Redistribution and use in source and binary forms, with or without | ||
modification, are permitted provided that the following conditions are met: | ||
Redistribution and use in source and binary forms, with or without modification, |
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.
What was wrong with the formatting of the LICENSE file?
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 wanted to adapt the same LICENSE file. Maybe this should be reverted back.
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 have no real idea how I ended up with a license version that is formatted differently to others. Can you make it a different commit though since it's not related to the pypi changes in the same 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.
Yes.
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.
Reverted
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 didn't say revert, I said put on a different commit on this branch.
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.
Yes, Reverted first with a forced push, added a separate commit. The LICENSE file is now the one from
lsst_versions
Operating System :: OS Independent | ||
Programming Language :: Python :: 3 | ||
Programming Language :: Python :: 3.8 | ||
Programming Language :: Python :: 3.9 |
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.
We probably need to start using tox and testing multiple python versions in a matrix (or at least not using tox but still using a matrix). I'm fine with you doing that on a later ticket.
bf84220
to
ffbb64a
Compare
BSD 3-Clause License | ||
|
||
Copyright (c) 2022, lsst | ||
All rights reserved. |
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.
Much nicer with the license version explicitly stated.
Checklist
doc/changes