3.0.0.pre.2 - ParameterType to replace Transform for domains

[JeanMertz]

We have the following transform:

Transform(/https\:\/\/blendle\.com(.*)/) do |path|
  'http://localhost:4545' + path
end

I tried rewriting it to a parameter type, while keeping the same functionality:

ParameterType(
  name: 'blendle.com',
  regexp: %r(https://blendle\.com(.*)),
  type: String,
  transformer: ->(path) { binding.pry; 'http://localhost:4545' + path },
  use_for_snippets: false,
  prefer_for_regexp_match: true
)

But it seems like the transformer is never triggered, and instead the original domain is visited. What am I doing wrong here?

[JeanMertz]

Alright, turns out the name has to be used in a cucumber expression, or (I guess?) it can match the regex defined in the step.

It does seem like this is not backward compatible, as the above Transform was not working with our step, which looked something like this:

Given(/^I visit "([^"]*)"$/) do |url|
end

With this, and the original Transform (which, as I can see in the code, converts it to a ParameterType), made the transformation no longer work.

This is not a big deal (it's a new major release, so backward breaking changes are to be expected), but I wanted to point this out nonetheless.

[mattwynne]

So @JeanMertz are you saying that when you upgraded, the existing Transform code stopped working (before you added the ParameterType?)

[JeanMertz]

@mattwynne that is correct

[aslakhellesoy]

It will work if you change your step definition from:

Given(/^I visit "([^"]*)"$/) do |url|

to:

Given(/^I visit "(https\:\/\/blendle\.com(.*))"$/) do |url|

The regexp inside the capture group must be identical to the regexp in the Transform.

I realise most people probably don't have this, so you're right - the new (deprecated( Transform implementation isn't 100% backwards compatible. We probably could make it 100% backwards compatible, but I have a feeling it might be a fair amount of work.

Because it isn't 100% backwards compatible I wonder if we should simply remove it and provide detailed instructions about how to migrate from Transform to ParameterType.

What do you think @JeanMertz @mattwynne ?

[aslakhellesoy]

My proposal is to remove Transform completely since we cannot easily preserve backwards compatibility.

Let me know what you think.

[mattwynne]

We could do a quick 2.5 (or 2.4.1 or 2.99?) release that deprecates Transform to warn people, then remove it in 3.0

As you say @aslakhellesoy a nice blog post explaining how to migrate would be useful too.

[mattwynne]

I'd imagine there will be a fair few people with them tucked away in their support directories so we want to be as kind to them as possible.

[danascheider]

Yeah I like the idea of adding a deprecation warning to a v2.x release and then axing it from version 3. I think one of the best aspects of 3.x is that we're going in a more opinionated direction and I have no problem making this part of that.

[JeanMertz]

If a deprecation warning is added before a 3.0 release, I’m sure that will help most people tackle this. Add to that an extra warning in the release notes and blog post, and I’m sure most will be fine.

It is a 3.0 release after all, so some backward incompatible changes are to be expected 👍

[aslakhellesoy]

Deprecation warnings should provide guidelines about what to do to get rid of the warning. As long as we don't provide an alternative I think a deprecation warning would just be annoying as there is no way to get rid of it.

I think the best option is to write that blog post, and point people to that from the 3.0.0 release notes.

[aslakhellesoy]

So here is the plan:

1. I'll publish my blog post Upgrading to Cucumber 3.0.0
2. I'll remove support for Transform on the master branch. No deprecation, but the CHANGELOG will point to that blog post.
3. Someone makes a 3.0.0 release
4. @matt publishes his blog post Cucumber 3.0.0 Release Announcement

[mattwynne]

@aslakhellesoy I think we could do a step 2.5 where we issue a deprecation warning for Transforms in a 2.99 release with a link to your blog post. That seems like it would be the kindest thing to do for our users. I can do that after your blog post is live.

[mattwynne]

Otherwise, I'm 👍 👍 🌮 for this plan

[aslakhellesoy]

If we do what you say, people who install Cucumber-Ruby 2.5 will see a deprecation warning for all their Transforms.

They'll find that annoying and look for a way to get rid of the deprecation warnings, only to discover there is no way to get rid of them while staying on 2.5. That's not helping anyone.

[mattwynne]

@JeanMertz OK If I close this issue since we seem to have reached a decision not to fix it?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.