godog icon indicating copy to clipboard operation
godog copied to clipboard

Use common packages from monorepo

Open vearutop opened this issue 3 years ago • 3 comments

Description

This PR changes imports of messages and gherkin to use common monorepo.

Motivation & context

Pros:

  • A small change to godog
  • Code coverage and lint are not affected

Cons:

  • Runtime double dependency on a large repo (one copy for gherkin/go/v20 and one for messages/go/v17)
  • Every further change of imports (e.g. to messages/go/v18) is technically a breaking change for godog as end-users may depend on messages directly while implementing advanced step definitions.

Runtime dependency on a monorepo seems like not a big deal, for example in GitHub Actions those packages are retrieved in 1-2 seconds.

Resolves #389.

Type of change

  • Refactoring/debt (improvement to code design or tooling without changing behaviour)
  • Breaking change (will cause existing functionality to not work as expected)

Note to other contributors

No notes.

Update required of cucumber.io/docs

No update.

Checklist:

  • [x] My code follows the code style of this project.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [ ] I have read the CONTRIBUTING document.
  • [ ] I have added tests to cover my changes.
  • [x] All new and existing tests passed.

vearutop avatar Aug 27 '21 13:08 vearutop

Codecov Report

Merging #429 (7eb0ba9) into main (239a424) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #429   +/-   ##
=======================================
  Coverage   81.57%   81.57%           
=======================================
  Files          26       26           
  Lines        2176     2176           
=======================================
  Hits         1775     1775           
  Misses        308      308           
  Partials       93       93           
Impacted Files Coverage Δ
formatters/fmt.go 100.00% <ø> (ø)
internal/formatters/fmt.go 72.72% <ø> (ø)
internal/formatters/fmt_base.go 86.17% <ø> (ø)
internal/formatters/fmt_cucumber.go 84.76% <ø> (ø)
internal/formatters/fmt_events.go 97.09% <ø> (ø)
internal/formatters/fmt_multi.go 91.66% <ø> (ø)
internal/formatters/fmt_pretty.go 80.56% <ø> (ø)
internal/formatters/fmt_progress.go 93.15% <ø> (ø)
internal/formatters/undefined_snippets_gen.go 41.46% <ø> (ø)
internal/models/feature.go 100.00% <ø> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 239a424...7eb0ba9. Read the comment docs.

codecov[bot] avatar Aug 27 '21 13:08 codecov[bot]

@vearutop Sorry it's taken me so long to give you feedback on this and #428.

One bit of context that might affect this is that we've decided to break up the monorepo, so the burden on users of downloading the dependencies will be much smaller once that's done.

Do you have a favourite of the two?

mattwynne avatar Oct 15 '21 23:10 mattwynne

Note that messages is well on the way to being removed.

mattwynne avatar Jul 12 '22 22:07 mattwynne

Closing this, as we need to migrate to different standalone modules (https://github.com/cucumber/common/issues/2029).

vearutop avatar Nov 10 '22 10:11 vearutop