search

leonelquinteros / gotext

Go (Golang) GNU gettext utilities package
Other
434 stars 58 forks source link

DRY up po/mo #43

Closed razor-1 closed 4 years ago

razor-1 commented 4 years ago

There's a fair amount of duplicate code in po.go and mo.go. Running golangci-lint with dupl enabled yields:

mo.go:279: 279-330 lines are duplicate of po.go:307-358 (dupl) po.go:307: 307-358 lines are duplicate of mo.go:279-330 (dupl) mo.go:449: 449-472 lines are duplicate of po.go:478-501 (dupl) po.go:478: 478-501 lines are duplicate of mo.go:449-472 (dupl)

I would propose some refactoring to move this common code into a new file, domain.go. I have done this successfully in a branch and it can be done with minimal changes to the public methods, though there may be enough change to warrant this being v2.

This will also allow cleaning up some duplicate tests.

leonelquinteros commented 4 years ago

Hi @razor-1 , Thanks for your contribution! I'd like to see your proposal as a PR whenever you can. I'm always up for a DRY-up, but not so sure about changing a public interface to improve the code.

Let's see the proposal. and discuss our options.

razor-1 commented 4 years ago

Thank you, I tried to minimize public interface changes in #44 as much as possible but there may still be room for improvement.

leonelquinteros commented 4 years ago

Merged!