Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Module refactoring #16

Merged
merged 3 commits into from
Nov 12, 2020
Merged

Conversation

creekorful
Copy link
Contributor

Hello there!

First of all thank you for this library, it has been really useful to me!

I have done a little refactoring to turn your library into a Go module, as well as moving the code library part into pkg directory (following github.com/golang-standards/project-layout standards)

Turning the library into a Go module allow us to remove the whole vendor directory. Which is the new language way to go. See https://blog.golang.org/modules2019

The only thing is, I think it would be a good idea to increase major version, since this change would break existing codebase.

What do you think ?

P-S: It should be great to add support for Go 1.12,1.13,1.14 (by adding them in Travis config file). I can do that if you want.

@coveralls
Copy link

coveralls commented Apr 14, 2020

Coverage Status

Coverage decreased (-5.3%) to 37.5% when pulling 2420b69 on creekorful:module-refactoring into 1f746af on sabhiram:master.

@BlackHole1
Copy link

This is great, is there any latest progress?
@sabhiram What do you think?

cmd/wol/wol.go Outdated
@@ -5,15 +5,14 @@ package main
import (
"errors"
"fmt"
"github.com/sabhiram/go-wol/pkg/wol"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : I always try to keep non standard packages after the first set of imports - but we can deal w/ this later 👍 Thank you for this work btw!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about this, I usually run gofmt over my files and it rearranges imports like this.

I can revert that if you want!

@sabhiram
Copy link
Owner

Yea if you would be kind enough to fix up travis for now - or we can move to github actions etc etc I will happily merge.

Also, we can release v2.0.0 in support of the modulization work you have done. Ill hold merging until you comment and ack this plan. Thank you again for the work and I am sorry I didnt see this till just now.

@creekorful
Copy link
Contributor Author

Hey @sabhiram !

Yea if you would be kind enough to fix up travis for now - or we can move to github actions etc etc I will happily merge.

Sure no problem. If you consider the move to Github Action then I'll do the change in the same PR!

Also, we can release v2.0.0 in support of the modulization work you have done. Ill hold merging until you comment and ack this plan.

I'll just do a little change: the pkg folder I have created is in fact not a good practice anymore (see here).
So I'll move files in root directory.

Thank you again for the work and I am sorry I didnt see this till just now.

No problem at all!

Cheers,

@creekorful
Copy link
Contributor Author

Hey back @sabhiram,
I have push another commit to move everything out of pkg folder (bad practice).

Regarding Github Action migration, I've noticed that your pipeline does a lot of stuff, and therefore I prefer doing the change in another PR to not bloat this one more.

Is that okay for you?

Cheers,

@sabhiram
Copy link
Owner

Ya totally fine by me, thank you for helping!

@sabhiram sabhiram merged commit 1f8eb2f into sabhiram:master Nov 12, 2020
@creekorful creekorful deleted the module-refactoring branch November 12, 2020 06:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants