awalterschulze / goderive

Derives and generates mundane golang functions that you do not want to maintain yourself
Apache License 2.0
1.24k stars 43 forks source link

add support for go modules #61

Closed jbl428 closed 3 years ago

jbl428 commented 3 years ago

Hi. Thank you for making this program. I love goderive. I always use this for all project. I want to contribute and I found that it use deprecated govendor. so I change some files to support go modules and it success tests (make travis on go version 1.16). Please review this pull request.

awalterschulze commented 3 years ago

We lost our CI, since we haven't moved to github actions yet. This has caused me to delay reviewing this diff, sorry about that.

Maybe as an intermittent step, may I ask if you have run make travis and whether this passes?

awalterschulze commented 3 years ago

Otherwise it looks like you have been thorough and I would like to merge this. What do you think?

awalterschulze commented 3 years ago

Thank you for going out of your way to fix this. Always great to hear about people enjoying to use this project :)

jbl428 commented 3 years ago

I have already run make travis and it haved passed. (go version 1.16).

But I'm afraid there might be compatibility issues.

I think I should change go version(1.16) in go.mod file for older go versions.

so, I want to go over one more and I will update this review.

awalterschulze commented 3 years ago

I think it is okay to only support the newest version of go, because people can "vendor" an older version. What do you think?

jbl428 commented 3 years ago

Yeah, You are right. and I found that it builds well on old version (1.11) even if latest version specified in go.mod. 😄

And I think it would be better to give some version tag to this project. then people can choose the right version.

awalterschulze commented 3 years ago

I agree, but to release a version, I think we should first fix the CI, but then that sounds like a great idea.

awalterschulze commented 3 years ago

In the mean time, are you happy if we merge this pull request?

jbl428 commented 3 years ago

Yes. Please accept this pull request.

Thank you for reviewing. 👍