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

Remove dependency to cucumber-wire #1562

Merged
merged 16 commits into from
Dec 6, 2021
Merged

Conversation

aurelien-reeves
Copy link
Contributor

@aurelien-reeves aurelien-reeves commented Jul 21, 2021

Description

cucumber-ruby-wire has been design to be an external optional dependency. This is not the case yet and it is responsible for adding complexity in the dependency tree for cucumber-ruby.

This PR aims to remove the dependency to cucumber-ruby-wire. The wire protocol would still be available but requiring it explicitly.

Type of change

  • Refactoring (improvements to code design or tooling that don't change behaviour)
  • Breaking change (will cause existing functionality to not
    work as expected)

Note to other contributors

Tests may be passing but there are a lot of things to do before merging this PR. We will certainly have to deprecate it first in a new major version before removing it completely in another new one.

For now what remains to be done here is to add a hook for cucumber-ruby-wire to be able to install by itself.

Checklist:

Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.

  • Tests have been added for any changes to behaviour of the code
  • New and existing tests are passing locally and on CI
  • bundle exec rubocop reports no offenses
  • RDoc comments have been updated
  • CHANGELOG.md has been updated
  • cucumber-ruby-wire is ready and work with this branch
  • The wire protocol has been deprectated

@aurelien-reeves
Copy link
Contributor Author

cucumber/cucumber-ruby-wire#46 is the cucumber-ruby-wire counter-part of this PR.

Everything seems to be ok actually, we would just need a deprecation/release plan for this but technically, it seems to be all good

@aurelien-reeves aurelien-reeves modified the milestones: 9.0.0, 8.0.0 Jul 27, 2021
@aurelien-reeves aurelien-reeves modified the milestones: 8.0.0, 9.0.0 Aug 5, 2021
@aurelien-reeves aurelien-reeves modified the milestones: 9.0.0, 8.0.0 Aug 27, 2021
@aurelien-reeves aurelien-reeves added the 🏦 debt Tech debt label Oct 13, 2021
@aurelien-reeves aurelien-reeves marked this pull request as ready for review November 3, 2021 13:44
@aurelien-reeves
Copy link
Contributor Author

We are ready to remove it now 😀

@aurelien-reeves
Copy link
Contributor Author

I am ready to merge that one if no further objection @mattwynne @luke-hill?

Copy link
Contributor

@luke-hill luke-hill left a comment

Choose a reason for hiding this comment

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

Minor typos. Other than that RtM

RELEASING.md Show resolved Hide resolved
UPGRADING.md Outdated Show resolved Hide resolved
UPGRADING.md Show resolved Hide resolved
@aurelien-reeves aurelien-reeves merged commit 532fcfd into main Dec 6, 2021
@luke-hill luke-hill deleted the remove-wire-dependency branch August 22, 2023 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏦 debt Tech debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants