search

influxdata / go-syslog

Blazing fast syslog parser
MIT License
476 stars 69 forks source link

Fixing go modules transition #22

Closed lxfontes closed 5 years ago

lxfontes commented 5 years ago

Users cannot use octet parser due to go module name inconsistency. Both develop branch and v2.0.0 tag ( the one with octet parser ) are not usable due to v2 being present in the module name.

This might be working fine locally / in development, but fails for external / Go dep users. This PR fixes all v2 mentions and can be validated with Go dep using

[[constraint]]
  name = "github.com/influxdata/go-syslog"
  branch = "develop"
  source = "https://github.com/lxfontes/go-syslog"
leodido commented 5 years ago

Hello @lxfontes thanks for noticing it and providing a fix!

Will look at it very soon

leodido commented 5 years ago

Yeah this looks pretty good to me. I forgot to comment, sorry about that.

The only question I have is whether to release this as 2.1 or override the 2.0 tag. I personally prefer the first option because retagging is not a good practice imho (in this case the interface is the same so it could also be done but still..).

On Thu, Aug 8, 2019, 11:55 PM Chris Goller notifications@github.com wrote:

@goller approved this pull request.

Hey @leodido https://github.com/leodido this looks good to me... are you ok with a merge?

@lxfontes https://github.com/lxfontes will we need to make a new v2 release?

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/influxdata/go-syslog/pull/22?email_source=notifications&email_token=AAA5J46KUPTKLUDVAZVKW6DQDSI3JA5CNFSM4IGRLP62YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBBX3FQ#pullrequestreview-272858518, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA5J4ZQOHXA4XH4BYVZ5ALQDSI3JANCNFSM4IGRLP6Q .

goller commented 5 years ago

Iā€™m with you, Leo. Seems like 2.1 is the way to go

On Aug 8, 2019, at 6:37 PM, Leo Di Donato notifications@github.com wrote:

Yeah thia looka pretty good to me. I forgot to comment sorry about that.

The only question I have is whether to release this as 2.1 or overrode the 2.0 tag. I personally prefer the first option because retagging is not a good practice imho (in this case the interface is the same so it could also be done but still..).

On Thu, Aug 8, 2019, 11:55 PM Chris Goller notifications@github.com wrote:

@goller approved this pull request.

Hey @leodido https://github.com/leodido this looks good to me... are you ok with a merge?

@lxfontes https://github.com/lxfontes will we need to make a new v2 release?

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/influxdata/go-syslog/pull/22?email_source=notifications&email_token=AAA5J46KUPTKLUDVAZVKW6DQDSI3JA5CNFSM4IGRLP62YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBBX3FQ#pullrequestreview-272858518, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA5J4ZQOHXA4XH4BYVZ5ALQDSI3JANCNFSM4IGRLP6Q .

ā€” You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

lxfontes commented 5 years ago

sorry didn't catch this earlier. @leodido feel free to open an issue and assign to me if you think this needs more work šŸ™Œ