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

Add --no-strip-extras and warn about strip extras by default #1954

Merged
merged 6 commits into from
Aug 8, 2023

Conversation

ryanhiebert
Copy link
Contributor

@ryanhiebert ryanhiebert commented Aug 6, 2023

References #1613. In preparation for the next major release where --strip-extras will be the default, give a warning if the flag is not given and add a matching --no-strip-extras flag to allow forcing the existing behavior without the warning.

Contributor checklist
  • Included tests for the changes.
  • PR title is short, clear, and ready to be included in the user-facing changelog.
Maintainer checklist
  • Verified one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

I haven't written tests yet, because I'm not clear on the failures. I'm opening this PR for review and to see what the tests look like in the pull request.

@ryanhiebert ryanhiebert added the deprecation Related to deprecations or removals label Aug 6, 2023
@ryanhiebert ryanhiebert added this to the 7.3.0 milestone Aug 6, 2023
@atugushev
Copy link
Member

@ryanhiebert check out #1955 where failures fixed.

Use stdout as an output in test_compile_recursive_extras
@atugushev atugushev changed the title Warn about strip extras by default Add --no-strip-extras and warn about strip extras by default Aug 8, 2023
@atugushev atugushev added enhancement Improvements to functionality and removed deprecation Related to deprecations or removals labels Aug 8, 2023
Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @ryanhiebert!

@ryanhiebert ryanhiebert merged commit b6193fd into main Aug 8, 2023
34 checks passed
@ryanhiebert ryanhiebert deleted the always-strip-extras-warning branch August 8, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants