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

wip: run deploy for PR #1891

Merged
merged 9 commits into from
Nov 18, 2022
Merged

wip: run deploy for PR #1891

merged 9 commits into from
Nov 18, 2022

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Nov 18, 2022


📚 The documentation for this PR will be available at https://awkward-array.readthedocs.io/en/agoose77-ci-fix-deploy-workflow/ once Read the Docs has finished building 🔨

@agoose77 agoose77 marked this pull request as draft November 18, 2022 10:51
@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #1891 (17d6211) into main (e943958) will not change coverage.
The diff coverage is n/a.

❗ Current head 17d6211 differs from pull request most recent head 502023f. Consider uploading reports for the commit 502023f to get more accurate results

Additional details and impacted files

@agoose77 agoose77 marked this pull request as ready for review November 18, 2022 13:18
@agoose77 agoose77 requested a review from jpivarski November 18, 2022 13:19
@agoose77
Copy link
Collaborator Author

agoose77 commented Nov 18, 2022

@jpivarski this commit tested the deployment for Python, whilst this commit tested the C++.

I installed the wheels using

cd $(mktemp-d)
python3 -m venv .venv
source .venv/bin/activate

# Populate wheels
mkdir wheels
python3 -m pip wheel packaging numpy pyparsing -w ./wheels
unzip <PATH-TO-CPP-WHEELS> -d wheels
unzip <PATH-TO-PY-WHEELS>  -d wheels

# Install awkward
python3 -m pip install --pre awkward --find-links=./wheels --no-index

# Test
python3 -c "import awkward as ak; builder=ak.ArrayBuilder();builder.integer(100);print(builder.snapshot())"

which succeeded. I was also able to build a C++ wheel from the C++ sdist, which passed the ArrayBuilder test above.

We can just merge this by skipping tests if you're happy to merge; we don't touch anything about from the deployment CI, which doesn't run in the test suite.

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

I approve!

name: "Make SDist"
name: Make SDist
Copy link
Member

Choose a reason for hiding this comment

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

I started indiscriminately quoting strings in YAML after being burned by some cases that looked like strings but weren't, due to some special character. It never hurts to quote. I don't think it's harder to read with quotes.

This one can be unquoted, of course.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I have the same philosophy. The Python versions always catch me out. I chose to make things look more uniform, but when writing new YAML I usually always quote.

- name: Compute SOURCE_DATE_EPOCH
shell: python
Copy link
Member

Choose a reason for hiding this comment

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

That's new to me. I guess you opted for Python as a shell because to use strip(). There's probably a shell equivalent, but why bother? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I'd rather not use Python! But, we run this on all OSs, and the Python implementation easily works on all platforms. We might want to make this a dev script, I'm half minded.

@agoose77 agoose77 force-pushed the agoose77/ci-fix-deploy-workflow branch from 502023f to 843886d Compare November 18, 2022 15:06
@agoose77 agoose77 merged commit 612fee0 into main Nov 18, 2022
@agoose77 agoose77 deleted the agoose77/ci-fix-deploy-workflow branch November 18, 2022 15:07
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