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

Introduce `original_system`, `original_exec`, `unbundled_system`, and `unbundled_exec` #7052

Merged
merged 1 commit into from Mar 27, 2019

Conversation

Projects
None yet
2 participants
@deivid-rodriguez
Copy link
Contributor

commented Mar 20, 2019

What was the end-user problem that led to this PR?

The problem was that the clean_system and clean_exec would print deprecation messages, but there was not alternative for them.

What was your diagnosis of the problem?

My diagnosis was that the helpers are using deprecated behavior and that we should provide non deprecated alternatives.

What is your fix for the problem, implemented in this PR?

My fix is to introduce original_system, original_exec, unbundled_system, and unbundled_exec for consistency with the rest of the helpers, and deprecate clean_system and clean_exec.

Why did you choose this fix out of the possible options?

I chose this fix because while maybe not super pretty names, they offer an alternative consistent with the other helpers.

Introduce `{original,unbundled}_{system,exec}` helpers
Because the previous helpers, `clean_exec` and `clean_system`, use
deprecated behavior and thus should be deprecated too.
@indirect
Copy link
Member

left a comment

This seems like the best possible solution to me. 👍

@indirect

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

bundlerbot bot pushed a commit that referenced this pull request Mar 27, 2019

Merge #7052
7052: Introduce  `original_system`, `original_exec`, `unbundled_system`, and `unbundled_exec`  r=indirect a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that the `clean_system` and `clean_exec` would print deprecation messages, but there was not alternative for them.

### What was your diagnosis of the problem?

My diagnosis was that the helpers are using deprecated behavior and that we should provide non deprecated alternatives.

### What is your fix for the problem, implemented in this PR?

My fix is to introduce `original_system`, `original_exec`, `unbundled_system`, and `unbundled_exec` for consistency with the rest of the helpers, and deprecate `clean_system` and `clean_exec`.

### Why did you choose this fix out of the possible options?

I chose this fix because while maybe not super pretty names, they offer an alternative consistent with the other helpers.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@bundlerbot

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

@bundlerbot bundlerbot bot merged commit 20dd665 into master Mar 27, 2019

3 checks passed

bors Build succeeded
Details
bundler.bundler Build #20190320.22 succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bundlerbot bundlerbot bot deleted the clean_env_pass branch Mar 27, 2019

bundlerbot bot pushed a commit that referenced this pull request Apr 1, 2019

Merge #7082
7082: Complete some missing specs r=deivid-rodriguez a=deivid-rodriguez

This is a follow up to #7052.

### What was the end-user problem that led to this PR?

The problem was that the `Bundle.with_clean_env` & `Bundle.with_original_env` family was missing some deprecation specs, and also the specs were spread out across different files.

### What is your fix for the problem, implemented in this PR?

My fix is to add the missing specs, and centralize them in the deprecation specs file.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.