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

REL: Prepare for 0.6.4 #1424

Merged
merged 7 commits into from
Aug 12, 2024
Merged

REL: Prepare for 0.6.4 #1424

merged 7 commits into from
Aug 12, 2024

Conversation

HaoZeke
Copy link
Member

@HaoZeke HaoZeke commented Aug 12, 2024

This is a bugfix release, so not much else, I think the steps here are what I'll need to follow post-merge. @lucascolley let me know if this seems like a reasonable amount of detail.

@HaoZeke HaoZeke requested a review from mattip August 12, 2024 15:42
@HaoZeke
Copy link
Member Author

HaoZeke commented Aug 12, 2024

I'm open to removing rangemedian from here in this release and moving it to a new repo, or that can also wait on the next, this one has some important changes (including the new .jsonc support)

@lucascolley
Copy link
Contributor

sounds good to me, thanks!

Copy link
Contributor

@mattip mattip left a comment

Choose a reason for hiding this comment

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

LGTM, just a few questions.

.tp_doc = NULL,
.tp_methods=RangeMedian_methods,
.tp_init=(initproc)RangeMedian_init,
.tp_new=RangeMedian_new
Copy link
Contributor

Choose a reason for hiding this comment

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

Over at cython, using this in c++ requires c20, so it errored out on windows with error C7555: use of designated initializers requires at least '/std:c++20'. Does it work here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Designated initializers is a C++20 feature but also C99 feature, so here this should be fine (this is also now the recommended style in the Python documentation). Although in the rewrite it would be much much better to cleanly separate the C++ parts (vector etc) from the Python bindings C.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually thinking about this a bit more I'm just going to revert the change here, there's a lot of weirdness here including the use of a template in a .cpp :'D

# if `name: artifact` is omitted, the action will create extra parent dir
name: artifact
path: dist
- uses: pypa/gh-action-pypi-publish@release/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you doing this differently now?

Copy link
Member Author

Choose a reason for hiding this comment

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

We never set up trusted publishing so this wasn't running, and shouldn't have been here. The release notes clarify this, explaining that we use twine for manual management.

I prefer manual management since you can test the wheels before uploading, the trusted publisher workflow means the only way to fix a bug is either a new release or a yank.

@HaoZeke HaoZeke requested a review from mattip August 12, 2024 18:32
@mattip mattip merged commit 8fe40a6 into airspeed-velocity:main Aug 12, 2024
12 checks passed
@HaoZeke HaoZeke deleted the rel64 branch August 12, 2024 20:48
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.

3 participants