warning messages added

[adbatista]

I added the warning messages

[tooky]

@adbatista thanks - that's great.

Don't worry about the failing feature - I think there's a brittle test that fails when new languages are added by gherkin.

Do you think it would be useful if the warning mentioned the non-mutating versions of those methods?

I'm also not sure what we'll be doing about #diff! - we'll need to provide a way to compare tables. We might be able to decorate the core AST::DataTable with a similar behaviour. @mattwynne @os97673 WDYT?

[adbatista]

@tooky
#map_column needs to be created or was moved to another class?
I will remove the warning from #diff! until decide what to do with it

[os97673]

Do you think it would be useful if the warning mentioned the non-mutating versions of those methods?

I think it would be nice to provide non-deprecated method name (if it exists) but this means you promise to keep those methods in 2.0. Do you already have list of those methods? If not let's keep just simple warnings and will update them as soon as you will have replacements in place.

Thus I'd suggest to apply the PR as is (and release the changes) but keep in mind that we will need to provide replacements in future. @mattwynne @tooky WDYT? If you agree I'm ready to merge/release the changes.

[tooky]

@os97673 yes that makes sense.

[os97673]

@adbatista I've looked at the code and it looks like #map_headers will print the warning too because it uses #map_headers! internally :(
Am I right or I've missed something?

[adbatista]

You're right @os97673
I saw the code, but I had not realized this problem
It will be necessary refactor #map_headers

[adbatista]

I think if I refactor #map_headers in this branch or create other one

[os97673]

I think if I refactor #map_headers in this branch or create other one

it is better to do this as part of the PR.
I'd introduce a new method with ugly name (something like temporary_substitution_for_map_headers!) which will contain all necessary code and both map_headers and map_headers! will call it.

[tooky]

@os97673 @adbatista you should just be able to create a new instance of the AST::Table in #map_headers passing the new mappings and conversion proc to the constructor, rather than calling dup then modifying the new instance.

[adbatista]

@tooky, @os97673
I did creating a new instance, you can see here #528

[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.