Skip to content

refactor!: only support winston3#297

Merged
JustinBeckwith merged 6 commits into
googleapis:masterfrom
ofrobots:winston3-only
Apr 16, 2019
Merged

refactor!: only support winston3#297
JustinBeckwith merged 6 commits into
googleapis:masterfrom
ofrobots:winston3-only

Conversation

@ofrobots
Copy link
Copy Markdown
Contributor

BREAKING CHANGE: Drop support for winston2 as a semver major change.

Ref: #278

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 10, 2019
@ofrobots
Copy link
Copy Markdown
Contributor Author

This unblocks #278. We may spin off winston2 support (if it is still necessary) into a separate package, or continue to support it from the penultimate semver major branch.

@ofrobots ofrobots added the semver: major Hint for users that this is an API breaking change. label Apr 10, 2019
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 10, 2019

Codecov Report

Merging #297 into master will increase coverage by 1.87%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
+ Coverage    92.3%   94.18%   +1.87%     
==========================================
  Files           6        4       -2     
  Lines         104       86      -18     
  Branches       18       15       -3     
==========================================
- Hits           96       81      -15     
+ Misses          4        2       -2     
+ Partials        4        3       -1
Impacted Files Coverage Δ
src/index.ts 81.81% <80%> (-3.9%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f27dab...b77031c. Read the comment docs.

Copy link
Copy Markdown
Contributor

@soldair soldair left a comment

Choose a reason for hiding this comment

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

looks good. i was confused by how many install tests were left that only changed winston2 to 3 but had the same ts source. but it makes sense because we made it comptable with both tests cases for winston 2 are important for either version.

Comment thread system-test/logging-winston.ts Outdated
Copy link
Copy Markdown
Contributor

@DominicKramer DominicKramer left a comment

Choose a reason for hiding this comment

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

I also don't have a problem dropping support for Winston 2 with the model of supporting in the future in a separate module or other mechanisms if there is customer need.

@ofrobots
Copy link
Copy Markdown
Contributor Author

@JustinBeckwith this is good to land, but an Administrator will have to force land this. Snyk is complaining about the removal of a package.json file. This is intentional.

@JustinBeckwith JustinBeckwith merged commit 9d3911b into googleapis:master Apr 16, 2019
@ofrobots ofrobots deleted the winston3-only branch April 16, 2019 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. semver: major Hint for users that this is an API breaking change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants