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.

Sign up for GitHub

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

Jump to bottom

Added cleaner Executrix temp file names API, deprecated array-based API #1037

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added cleaner Executrix temp file names API, deprecated array-based API #1037

wants to merge 1 commit into from

Conversation

drivenflywheel
Copy link
Collaborator

No description provided.

@jpdahlke jpdahlke added the improvement A change that makes something easier to use label Jan 6, 2025
@rpg36
Copy link
Collaborator

rpg36 commented Jan 8, 2025

This makes me happy! Anything to improve executrix!

dev-mlb
dev-mlb approved these changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@dev-mlb dev-mlb left a comment

Choose a reason for hiding this comment

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

I like this change. The only thing I'd maybe change is possibly hold off on the *CommandPlaces -- get the changes in and then activate them in a follow up. I could be convinced either way.

rpg36
rpg36 approved these changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@rpg36 rpg36 left a comment

Choose a reason for hiding this comment

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

I have no preference personally as to if this should be broken up into 2 phases or not as @dev-mlb suggested.

@jpdahlke jpdahlke added this to the v8.21.0 milestone Jan 10, 2025
@drivenflywheel
Copy link
Collaborator Author

I like this change. The only thing I'd maybe change is possibly hold off on the *CommandPlaces -- get the changes in and then activate them in a follow up. I could be convinced either way.

I thought about that too, but these changes to both Places are method-local. Transparent to downstream, which integrates this with zero code changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dev-mlb dev-mlb dev-mlb approved these changes

@rpg36 rpg36 rpg36 approved these changes

Assignees
No one assigned
Labels
improvement A change that makes something easier to use
Projects
None yet
Milestone
v8.21.0
Development

Successfully merging this pull request may close these issues.
4 participants
@drivenflywheel @rpg36 @dev-mlb @jpdahlke