Merge context package into the main package

go-bdd / gobdd

BDD framework

https://go-bdd.github.io/gobdd/

MIT License

115 stars 19 forks source link

Merge context package into the main package #104

Closed sagikazarmark closed 4 years ago

sagikazarmark commented 4 years ago

Fixes #96

This is a fully backwards compatible change by adding aliases for the necessary type (Context).

ToDos

[ ] NewContext does not have to be exported. Should it be?

Possible alternatives

Introduce an interface: that would be a somewhat backward incompatible change, but the Context should only be used by consumers as a type, it shouldn't be manually instantiated. (An interface can still be added after this PR)
Add a type alias to the context package: would be less code change, but eventually the context package should go away

bkielbasa commented 4 years ago

thanks for your contribution!

I think we can remove the context package in v2.0.

IMO we can make NewContext unexported.

sagikazarmark commented 4 years ago

Oh, it looks like we cannot make NewContext unexported, if we want to keep New in the context package.

bkielbasa commented 4 years ago

please fix linter issues and I can merge it

sagikazarmark commented 4 years ago

One of the failing lint rules seems to be false positive.

sagikazarmark commented 4 years ago

gobdd.go:415:38: `t` can be `github.com/stretchr/testify/assert.TestingT` (interfacer)
func (def *stepDef) run(ctx Context, t TestingT, params [][]byte) {

I think this is false positive. I can either disable linting in this line with // nolint: interfacer, or disable the interfacer linter entirely.

bkielbasa commented 4 years ago

@sagikazarmark let's disable it for this line for now. If we'll have more problems like that we can disable it entirely.

bkielbasa commented 4 years ago

@sagikazarmark you don't have to rebase commits because I squash and merge them anyway :)

bkielbasa commented 4 years ago

Thanks for your contribution!