Skip to content

Conversation

@ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Jul 7, 2021

Nothing to see here for now. Just testing...

@codecov
Copy link

codecov bot commented Jul 7, 2021

Codecov Report

Merging #186 (899d9ef) into main (5540e08) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #186   +/-   ##
=======================================
  Coverage   90.96%   90.96%           
=======================================
  Files           6        6           
  Lines         819      819           
  Branches      103      103           
=======================================
  Hits          745      745           
  Misses         62       62           
  Partials       12       12           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5540e08...899d9ef. Read the comment docs.

@bjlittle
Copy link
Member

bjlittle commented Jul 8, 2021

@ocefpaf I was about to crank out the bdist_wheels for cf-units manually... but if we can automate this, it would be lovely 🤩

@bjlittle bjlittle marked this pull request as draft July 8, 2021 14:37
@ocefpaf ocefpaf marked this pull request as ready for review July 8, 2021 19:22
@ocefpaf ocefpaf marked this pull request as draft July 8, 2021 19:32
@bjlittle bjlittle self-assigned this Jul 8, 2021
@bjlittle bjlittle added New: Pull Request Highlight a new community raised pull-request Type: Enhancement Type: Infrastructure labels Jul 8, 2021
@ocefpaf
Copy link
Member Author

ocefpaf commented Jul 8, 2021

@bjlittle I'm not going to work on Windows and macOS at the moment b/c those require some extra knowledge and time that I don't have. (Installing udunits2 in those systems, finding the right paths, etc.)

This is almost ready but there is a problem with the udunits2.xml file. We rely on site.cfg and a fixed path for it. For the wheel we should copy that file with the distribution and change the code to find it. Or I'm just not thinking of a nice pythonic way to define an env var for site.cfg that aims into the installed directory.

TL;DR I'm not going to dig into this b/c code changes here would imply extra tests and adaptations to avoid breakages.

Do you have any insights on how to solve this?

@bjlittle
Copy link
Member

bjlittle commented Jul 28, 2021

@ocefpaf Test away my man... ping me when you think this PR is good to go.

Take it out of draft and I'll re-review again... it's mighty close now 🍻

@ocefpaf ocefpaf marked this pull request as ready for review July 28, 2021 23:59
@ocefpaf
Copy link
Member Author

ocefpaf commented Jul 29, 2021

Screenshot from 2021-07-28 20-58-07

PS: I'll work on macOS and Windows at a later time. Linux was already quite the battle for now 😄

uses: pypa/gh-action-pypi-publish@master
with:
user: __token__
password: ${{ secrets.PYPI_PASSWORD }} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

I created an API token on PyPI for a generic scitools-pypi user that we have, and also created the associated cf-units GitHub repository secret PYPI_PASSWORD

python -m twine check *

- name: Publish a Python distribution to PyPI
uses: pypa/gh-action-pypi-publish@master
Copy link
Member

@bjlittle bjlittle Jul 29, 2021

Choose a reason for hiding this comment

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

We should pin this to a specific stable release version, say @release/v1.4.2 which is the latest.

But I'll do that as a separate PR and ping you on that... I think that you've been through enough already

pypi-publich-github-action

Copy link
Member

@bjlittle bjlittle left a comment

Choose a reason for hiding this comment

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

Awesome, let's go for it @ocefpaf 🚀

I'll create a follow-up PR to pin the action release version used 👍

@bjlittle bjlittle merged commit 4f05940 into SciTools:main Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New: Pull Request Highlight a new community raised pull-request Type: Enhancement Type: Infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants