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

Update cucumber/common module references to use paths instead of subrepos #389

Open
mattwynne opened this issue May 21, 2021 · 7 comments
Open
Assignees
Labels
🏦 debt Tech debt 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8

Comments

@mattwynne
Copy link
Member

We have an initiative (cucumber/common#1550) to remove some technical debt in the cucumber/common repo around publishing read-only subrepos from the monorepo. We'd like to remove/archive these read-only repos altogether, and the scripts that copy code into them.

As part of this, we need to update godog to reference the common modules (gherkin etc) via a path within the common repo, rather than the subrepo. See @aurelien-reeves's comment here.

I'm not all that familiar with the go ecosystem, but I can probably figure out a PR for this.

What I'm not sure is about the deployment process. If we make a release of godog with the new module paths, will end users have to upgrade if we then archive the subrepos?

@mattwynne mattwynne added 🏦 debt Tech debt 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8 labels May 21, 2021
@mattwynne mattwynne added this to In Progress in Cucumber Open May 21, 2021
@mattwynne mattwynne removed this from In Progress in Cucumber Open May 21, 2021
@aslakhellesoy
Copy link
Contributor

Step one would be to see if we can get gherkin/go building by referencing messages from the monorepo.

@mattwynne
Copy link
Member Author

OK I've had a go at that here but I could do with some help from someone who has some idea what they're doing with go. I have none.

@mattwynne
Copy link
Member Author

@vearutop any chance you could help me with this?

It's blocking a larger piece of work to decommission a bunch of obsolete scripts to maintain the mirror repos from cucumber/common.

@vearutop
Copy link
Member

Sure, let's do this after releasing v0.12.0.

@vearutop
Copy link
Member

vearutop commented Aug 28, 2021

I've created a couple of draft PR for this issue, one is a straightforward imports replace as suggested by this issue.
And another is a more complicated solution to embed dependencies into internal packages of godog.

I don't have a strong opinion which approach is the best, both have pros and cons.

Let's decide which way to go and I can finish one of the drafts with documentation/changelog update.

@mpkorstanje
Copy link
Contributor

I think you have spotted the same problem I've been trying to deal with in Cucumber-JVM.

The messages and gherkin dependencies are exposed as part of cucumbers api. This is made worse by the dependency structure in which messages is a dependency of everything. As a result updates to for example the html formatter often force a new major version.

In Cucumber-JVM I have chosen not to expose these dependencies. This was made easier by already having an existing API. This API uses an adapter pattern to shield users from rapid changes.

Is it possible do something similar in godog without also copying the dependencies to an internal module?

@vearutop
Copy link
Member

The main leak of foreign API is messages, godog uses local aliases to refer messages types which helps against import path changes. If there is a breaking change (renamed property for example) in messages, that would be technically a breaking change for godog. Foreign types (as local aliases) are used in step definitions and custom formatters.

In practice, upgrades to newer messages don't break much of the end-user code, so maybe it is ok to keep direct exposition of foreign API at some maintenance cost for the end-user.

As you suggested from Cucumber-JVM experience, we can stabilize godog by introducing own structures for messages and implementing adapters to common messages. That would be a moderate amount of refactoring but maybe that would help larger adoption due to improved stability.

gherkin dependency is only used in internal packages, so it is not exposed directly and does not propagate breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏦 debt Tech debt 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8
Projects
None yet
4 participants