Skip to content
This repository has been archived by the owner on Oct 16, 2018. It is now read-only.

Initial Commit #1

Merged
merged 8 commits into from
Mar 21, 2017
Merged

Initial Commit #1

merged 8 commits into from
Mar 21, 2017

Conversation

prateek
Copy link
Collaborator

@prateek prateek commented Mar 15, 2017

Copy link

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

Need to add the license to all source files.

@@ -0,0 +1,15 @@
language: go

Choose a reason for hiding this comment

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

Not something we have to solve with this PR but I wonder if we should templatize this file somehow so we don't have to change all of them each time we want to support different versions of Go for CI.

It could even be something like when running in CI it checks whether the versions of Go match the "master" version in some shared CI repo or something and fails the test run if it it's not up to date.

Let's forget doing anything for this repo, but wanted to bring it up. Let's find a way to solve this in the near future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool, opened m3db/ci-scripts#9 to track.

@@ -0,0 +1,37 @@
package: github.com/m3db/m3nsch

Choose a reason for hiding this comment

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

Can we add versions to all modules here to pin the modules? Just so we don't get random upgrades to packages unless we specify so. Pinning to at least a semver to avoid running glide update and getting a breaking change from packages not meant to be upgraded is important.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For sure. I was under the impression that versions being mentioned in glide.lock was sufficient, will put them in glide.yaml too.

Copy link
Collaborator 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 88708c0 on prateek/initial-commit into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8387db8 on prateek/initial-commit into ** on master**.

@prateek
Copy link
Collaborator Author

prateek commented Mar 21, 2017

@r added licence in each file

@prateek
Copy link
Collaborator Author

prateek commented Mar 21, 2017

will open a subsequent pr to address the low coverage

@prateek prateek mentioned this pull request Mar 21, 2017
Copy link

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

LGTM

@prateek prateek merged commit fe99754 into master Mar 21, 2017
@prateek prateek deleted the prateek/initial-commit branch March 21, 2017 03:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants