skpkg: setup CI after migrating tests, src, requirements, and .github folder #80
Conversation
There was a problem hiding this comment.
Great to see this! Thanks for doing it @tinatn29
Please see inline comments. Here are some general thoughts.
- you seem to have deleted
tests-on-pr.ymlwhic we don't want to do. this runs our CI. - Are there instructions for "updating package" that are different from 'migrating legacy code'? If there are (I am not sure if they were finalized and/or posted but we can find out form @bobleesj and @cadenmyers13 ) but if they are available, we want to be following them here. They will tell you that we want to extend dates, not replace them, and also delete template functions in the docs/tests. These are put there to "seed" a new project like
function.pyandtest_functions.py.
I also see a pre-commit fix that was done in the CI. Please make sure you have pre-commit running locally to catch as many of these on your local before they reach the CI.
Other than this, this is great!
But all in all
src/diffpy/__init__.py
Outdated
| @@ -1,7 +1,7 @@ | |||
| #!/usr/bin/env python | |||
| ############################################################################## | |||
| # | |||
| # (c) 2022-2025 The Trustees of Columbia University in the City of New York. | |||
| # (c) 2025 The Trustees of Columbia University in the City of New York. | |||
There was a problem hiding this comment.
we would like to keep the full date range here, so 2022-2025
src/diffpy/fourigui/__init__.py
Outdated
| @@ -1,10 +1,10 @@ | |||
| #!/usr/bin/env python | |||
| ############################################################################## | |||
| # | |||
| # (c) 2022-2025 The Trustees of Columbia University in the City of New York. | |||
| # (c) 2025 The Trustees of Columbia University in the City of New York. | |||
src/diffpy/fourigui/version.py
Outdated
| @@ -1,13 +1,13 @@ | |||
| #!/usr/bin/env python | |||
| ############################################################################## | |||
| # | |||
| # (c) 2022-2025 The Trustees of Columbia University in the City of New York. | |||
| # (c) 2025 The Trustees of Columbia University in the City of New York. | |||
@sbillinge no. We don't have it. Since @cadenmyers13 and others are actively recutting, maybe you could make one? |
I won't be able to get to this personally for some time. I think Ethan was discussing it. @cadenmyers13 do you remember? |
|
@sbillinge I don't remember (@ycexiao can you confirm Simon's comment?). fyi i tried running |
|
@sbillinge @cadenmyers13 I don't know how |
There was a problem hiding this comment.
everything looks good now that the CI is back. We can either close this and redo everything as we discussed in the other PR (it is a bit worrying that the .github workflows disappeared before, so it may be safer to redo in case anything else is wrong). I could also merge this as most of the fixes look good, I recognize them! Before I can merge it we need to remove the function.py and test_function.py files using git rm (which will remove them from the git database and the file system)
I think we don't have the recutting instructions. But we do have a talk mentioning this in scikit-package/scikit-package#527. Our final decision is to implement commands like |
|
closing this one since I'm working on a new PR |
No description provided.