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 uplelvel to deprecation warning of Psych #1992

Closed
wants to merge 3 commits into
base: trunk
from

Conversation

3 participants
@koic

koic commented Oct 22, 2018

This PR adds uplelvel to deprecation warning of Psych.

Summary

The deprecation warning log has been added the following commit.
1c92766

The following is deprecation warning log change.

Example code

% cat /tmp/psych_example.rb
require 'psych'

Psych.load("--- foo\n", nil)

Before

% ruby -v
ruby 2.6.0dev (2018-10-21 trunk 65252) [x86_64-darwin17]

% ruby /tmp/psych_example.rb
warning: Passing filename with the 2nd argument of Psych.load is
deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.

After

This patch helps detect argument locations that are deprecated usage.

% cd /path/to/ruby/repo
% make install
% /usr/local/bin/ruby /tmp/psych_example.rb
/tmp/psych_example.rb:3: warning: Passing filename with the 2nd
argument of Psych.load is deprecated. Use keyword argument like
Psych.load(yaml, filename: ...) instead.

Other Information

This log format refers to the deprecation warning of ERB.new in Ruby 2.6+.
https://github.com/ruby/ruby/blob/v2_6_0_preview2/lib/erb.rb#L808

@koic koic force-pushed the koic:add_uplevel_to_deprecation_log_of_psych branch 2 times, most recently from 7507cfe to 8fc2654 Oct 22, 2018

@@ -270,7 +270,7 @@ module Psych
#
def self.load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false, symbolize_names: false
if legacy_filename != NOT_GIVEN
warn 'warning: Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.'
warn 'Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.', uplevel: 1

This comment has been minimized.

@koic

koic Oct 23, 2018

$VERBOSE has been added to the showing condition to deprecation warnings in ERB.new.
The following is a similar change using keyword arguments.
https://github.com/ruby/ruby/blob/v2_6_0_preview2/lib/erb.rb#L808

If necessary, I'm going to add $VERBOSE condition to these deprecation warnings.

- warn 'Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.', uplevel: 1
+ warn 'Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE

This comment has been minimized.

@hsbt

hsbt Nov 2, 2018

Member

Adding $VERBOSE looks good.

This comment has been minimized.

@koic

koic Nov 14, 2018

Thanks. I updated it.

@hsbt

I prefer this changes. But I'm not sure to add uplevel is working with under the Ruby 2.6. Can you try this change in ruby/psych repo?

@@ -270,7 +270,7 @@ module Psych
#
def self.load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false, symbolize_names: false
if legacy_filename != NOT_GIVEN
warn 'warning: Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.'
warn 'Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.', uplevel: 1

This comment has been minimized.

@hsbt

hsbt Nov 2, 2018

Member

Adding $VERBOSE looks good.

koic added some commits Oct 22, 2018

Add uplelvel to deprecation warning of Psych
This PR adds uplelvel to deprecation warning of Psych.

## Summary

The deprecation warning log has been added the following commit.
1c92766

The following is deprecation warning log change.

### Example code

```console
% cat /tmp/psych_example.rb
require 'psych'

Psych.load("--- foo\n", nil)
```

### Before

```console
% ruby -v
ruby 2.6.0dev (2018-10-21 trunk 65252) [x86_64-darwin17]

% ruby /tmp/psych_example.rb
warning: Passing filename with the 2nd argument of Psych.load is
deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.
```

### After

This patch helps detect argument locations that are deprecated usage.

```console
% cd /path/to/ruby/repo
% make install
% /usr/local/bin/ruby /tmp/psych_example.rb
/tmp/psych_example.rb:3: warning: Passing filename with the 2nd
argument of Psych.load is deprecated. Use keyword argument like
Psych.load(yaml, filename: ...) instead.
```

## Other Information

This log format refers to the deprecation warning of `ERB.new` in Ruby 2.6+.
https://github.com/ruby/ruby/blob/v2_6_0_preview2/lib/erb.rb#L808
Emulate `warn 'message', uplevel: 1` for Ruby 2.4 or lower
Follow up of #1992 (review).

The `uplevel` option was introduced from Ruby 2.5.
ruby/psych needs to support Ruby 2.4 or lower.
This commit emulates `warn 'message', uplevel: 1` in Ruby 2.4 or lower.

@koic koic force-pushed the koic:add_uplevel_to_deprecation_log_of_psych branch from 8fc2654 to a4f4108 Nov 14, 2018

@koic

This comment has been minimized.

koic commented Nov 14, 2018

I prefer this changes. But I'm not sure to add uplevel is working with under the Ruby 2.6. Can you try this change in ruby/psych repo?

I confirmed it. So it did not work as expected in Ruby 2.4 or lower. I update this PR. The following commit emulates warn 'message', uplevel: 1 in Ruby 2.4 or lower.
a4f4108

@koic

This comment has been minimized.

koic commented Nov 20, 2018

The complexity of code is pointed out with Code Climate, but it seems to be difficult to reduce code and reduce complexity. Even with refactoring, it seems difficult to make it more clear code now.

koic added a commit to koic/psych that referenced this pull request Nov 23, 2018

Add uplelvel to deprecation warning of Psych
This is porting ruby/ruby#1992 to upstream.

This PR adds `uplelvel` to deprecation warning of Psych.
The `uplevel` option was introduced from Ruby 2.5.
ruby/psych needs to support Ruby 2.4 or lower.

This PR has `warn_with_uplevel` method emulating
`warn 'message', uplevel: 1` in Ruby 2.4 or lower.

And this PR relaxes the warning.
ruby/ruby#1992 (comment)

## Summary

The deprecation warning log has been added the following commit.
ruby/ruby@1c92766

The following is deprecation warning log change.

### Example code

```console
% cat /tmp/psych_example.rb
require 'psych'

Psych.load("--- foo\n", nil)
```

### Before

```console
% ruby -v
ruby 2.6.0dev (2018-10-21 trunk 65252) [x86_64-darwin17]

% ruby /tmp/psych_example.rb
warning: Passing filename with the 2nd argument of Psych.load is
deprecated. Use keyword argument like Psych.load(yaml, filename: ...)
instead.
```

### After

This patch helps detect argument locations that are deprecated usage.

```console
% cd /path/to/ruby/repo
% make install
% /usr/local/bin/ruby /tmp/psych_example.rb
/tmp/psych_example.rb:3: warning: Passing filename with the 2nd
argument of Psych.load is deprecated. Use keyword argument like
Psych.load(yaml, filename: ...) instead.
```

## Other Information

This log format refers to the deprecation warning of `ERB.new` in Ruby 2.6+.
https://github.com/ruby/ruby/blob/v2_6_0_preview2/lib/erb.rb#L808
@koic

This comment has been minimized.

koic commented Nov 23, 2018

Can you try this change in ruby/psych repo?

I opened a PR ruby/psych#379.

@koic

This comment has been minimized.

koic commented Dec 4, 2018

I close this PR because ruby/psych upstream has been merged into the master.
631086b

@hsbt Thank you always!

@koic koic closed this Dec 4, 2018

@koic koic deleted the koic:add_uplevel_to_deprecation_log_of_psych branch Dec 4, 2018

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