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

Init impl #1

Merged
merged 22 commits into from
May 17, 2017
Merged

Init impl #1

merged 22 commits into from
May 17, 2017

Conversation

iulianp26
Copy link
Contributor

@iulianp26 iulianp26 commented May 9, 2017

Read from the video content "related" field, creates a story package container for related items and forwards the new message for further ingestion in neo4j.

See more documentation about Next Video editor processing here: https://sites.google.com/a/ft.com/universal-publishing/documentation/story-specs/3-mar-2017-next-video-platform-work

Tested in video cluster.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 500d488 on init-impl into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c223b33 on init-impl into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 390e9c7 on init-impl into ** on master**.

Dockerfile Outdated
&& go build -ldflags="${LDFLAGS}" \
&& mv ${PROJECT} /${PROJECT} \
&& apk del .build-dependencies \
&& rm -rf $GOPATH /var/cache/apk/*
Copy link

Choose a reason for hiding this comment

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

Could you please remove also /usr/local/go and $GOPATH/src?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

README.md Outdated
### Logging

* The application uses [logrus](https://github.com/Sirupsen/logrus); the log file is initialised in [main.go](main.go).
* Logging requires an `env` app parameter, for all environments other than `local` logs are written to file.
Copy link

Choose a reason for hiding this comment

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

Is this still valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed logging details.

@tosan88
Copy link

tosan88 commented May 12, 2017

Otherwise lgtm

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6e79e96 on init-impl into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b22ab14 on init-impl into ** on master**.

mapper.go Outdated
return nil, "", err
}

uuid := uuidUtils.NewV5UUIDFrom(videoUUID)

Choose a reason for hiding this comment

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

We should use the Deriver when generating an UUID based on another UUID: https://github.com/Financial-Times/uuid-utils-go/blob/master/deriveUUID.go#L27

Let's discuss about this a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 4d1ec18 on init-impl into ** on master**.

@ioanaciont ioanaciont merged commit efd87cb into master May 17, 2017
@tosan88 tosan88 deleted the init-impl branch May 18, 2017 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants