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

Integrate text on APE 5 and departures from it #2

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

jeffjennings
Copy link
Owner

In response to comments raised in astropy#112, this PR adds text to place APE26 in the context of APE5 and explicitly note departures from the approach/implementation in APE5.

@jeffjennings jeffjennings changed the title integrate text on APE 5 and departures from it Integrate text on APE 5 and departures from it Feb 10, 2025
@jeffjennings
Copy link
Owner Author

@adrn @mhvk @nstarman - I don't think this should be controversial, but I've made some minor text changes to address most of Erik's review comments - feel free to suggest changes or thumbs up for merging into the APE. Thanks!

Copy link

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

@jeffjennings - this looks all good to me! Though obviously the person really to ask is @eteq (Erik: this PR has changes following your larger comment about integration with APE 5).

@jeffjennings
Copy link
Owner Author

Great, thanks Marten! (Exactly, I was just waiting to leave some inline replies to @eteq's comments in the main APE PR; I'll merge this and leave them now.)

@jeffjennings jeffjennings merged commit 46a6d82 into APE26 Feb 10, 2025
@jeffjennings jeffjennings deleted the text_edits_ape5 branch February 10, 2025 20:58
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