Merged
Conversation
By setting a flag on the console logger, we can easily convert all outgoing logs with pelias-logger to a JSON format. Many of our log statements already use structured formats, and by printing them in JSON, they can all be readily parsed by almost any log-reading system (like Kibana, Honeycomb, Splunk, etc). In my testing, Winston essentialy checks if the flag is set to any truthy value so `true`, "true", 3, "maybe" and anything else except `null` and `false` will enable this feature. Since it's quite robust and handles any valid value, it does not seem necessary to add any validation ourselves.
This change moves semantic-release out of dev-dependencies, but keeps its functionality by calling semantic-release as usual in a `release` TravisCI build stage. There are several advantages to this method: 1.) semantic-release is run only after all builds succeed. Our previous approach could have theoretically run semantic-release when some Node.js versions failed with the current code 2.) semantic-release (and it's many dependencies) are removed from `node_modules`. This increases the speed of `npm install` in all cases, and reduces the size of our Docker images by 20MB (from 284MB to 264MB)! Since the only time semantic-release is needed is on TravisCI anyway, it seems pointless that every installation of Pelias should include it. 3.) Because semantic-release is not in `package.json`, Greenkeeper will not attempt to update it. Semantic release updates _very_ frequently, and each update attempt seems to have a decent chance of experiencing a random TravisCI failure, causing unwanted notifications. There are probably downsides to this approach. For example, we should consider pinning the major version of semantic release during install. Additionally, and for obvious reasons, we can't fully test this change until it's merged to the `production` branch. We should consider testing it first on a lower priority repository. If this change _does_ work well, we should consider adopting it everywhere.
As this is an NPM module, the master branch is where we do releases. Running semantic-release anywhere else just wastes time on TravisCI.
f760de2 to
5d7d447
Compare
Member
Author
|
Oops, this contained a feature i didn't mean it to. I reverted it from the master branch since I caught it right away. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This uses a new central CI script to run semantic release without putting it in dev-dependencies. See pelias/api#1187 for the full explanation.