Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Mixcloud bee #199

Merged
merged 11 commits into from
May 24, 2019
Merged

Mixcloud bee #199

merged 11 commits into from
May 24, 2019

Conversation

Horrendus
Copy link
Contributor

MixcloudBee that can follow a Mixcloud user/feed and creates an event when a new CloudCast was uploaded

As it is one of my first serious go & beehive contributions I leave it atm as WIP for review. Please also review the new dependency https://github.com/Horrendus/go-mixcloud

@muesli
Copy link
Owner

muesli commented Apr 26, 2019

Nice work! The first thing that pops out when looking at go-mixcloud, is that you moved the actual code in a sub-dir (mixcloud) and separated the tests into their own package.

This can be done (especially if you want to write your tests like an external package user), but the common approach is to move them along side the actual code (so you would have somefile.go and somefile_test.go). That way godoc can also extract examples from your docs.

Either way, I'd recommend moving the actual code directly into the go-mixcloud directory. Each Go package is only composed of one directory, so a subdirectory tests wouldn't mess things up.

@muesli
Copy link
Owner

muesli commented Apr 26, 2019

I also heavily recommend using gofmt and set up a hook in your IDE to automatically run it on saving a Go file. It's thoroughly good.

@Horrendus
Copy link
Contributor Author

I think I applied all your review comments on go-mixcloud, the mixcloud bee is not yet updated (need to read about go modules :) ).

For further review of the dependency I opened this issue here: Horrendus/go-mixcloud#1

Further review of the mixcloud bee should of course still be done here once I updated it ;)

@muesli
Copy link
Owner

muesli commented Apr 27, 2019

The good news is: Go modules doesn't change much for you right now.

Make sure to use Go 1.11+, and either set GO111MODULE=on, or compile beehive outside your $GOPATH. This will activate Go modules support. Now, if you import go-mixcloud as a dependency in your bee, Go will automatically detect that and update the go.mod file for you. Make sure to include the changes to that file in your pull request.

"The go command resolves imports by using the specific dependency module versions listed in go.mod. When it encounters an import of a package not provided by any module in go.mod, the go command automatically looks up the module containing that package and adds it to go.mod, using the latest version"

@Horrendus Horrendus changed the title WIP: Mixcloud bee Mixcloud bee Apr 30, 2019
@muesli
Copy link
Owner

muesli commented May 1, 2019

Looking good!

@Horrendus
Copy link
Contributor Author

The Bee no longer sleeps, it now has an action "pollFeed" that can be triggered by e.g. a CronBee. Not fully sure if I correctly did the options part of the action (Action shouldnt have any option).

Copy link
Collaborator

@CalmBit CalmBit left a comment

Choose a reason for hiding this comment

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

Just two tiny details that are probably largely inconsequential - otherwise, looking great!

bees/mixcloudbee/mixcloudbee.go Outdated Show resolved Hide resolved
bees/mixcloudbee/mixcloudbee.go Show resolved Hide resolved
@Horrendus
Copy link
Contributor Author

@CalmBit: Thanks for the suggestions!

@muesli
Copy link
Owner

muesli commented May 24, 2019

LGTM! Just waiting for TravisCI to finish.

@muesli muesli merged commit 5e1ccac into muesli:master May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants