Skip to content

chore: export core-logger-winston types.#1887

Merged
faustbrian merged 3 commits intoArkEcosystem:developfrom
paroxysm:feat/export-core-logger-winston-types
Dec 20, 2018
Merged

chore: export core-logger-winston types.#1887
faustbrian merged 3 commits intoArkEcosystem:developfrom
paroxysm:feat/export-core-logger-winston-types

Conversation

@paroxysm
Copy link
Copy Markdown
Contributor

Proposed changes

Further addresses #1795. While we're exporting the Logger type defined in this module. The rest of the modules will only depend on core-logger module for its AbstractLogger type. This ensures that, even though we're introducing a core-logger dependency to all modules that need to log(so as to benefit from type-safety), we still retain the ability to supplant the core-logger-winston implementation with a different implementation as long as that implementation extends from core-logger's AbstractLogger

i.e. Override the winston logger with a logstash json encoded logger or RDMS logger or even a network streaming logger. As long as all these extend from the AbstractLogger in core-logger module, they still benefit from the type-safety but don't restrict the rest of the modules to one specific implementation.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build (changes that affect the build system)
  • Docs (documentation only changes)
  • Test (adding missing tests or fixing existing tests)
  • Other... Please describe:

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

* @return {void}
*/
public abstract printTracker(title: string, current: number, max: number, postTitle: string, figures: number): void;
public abstract printTracker(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did tslint format it like that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Odd, never yet complained to me about that line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes it is...But I've noticed my IDE doesn't complain about max-len violations..
However, yarn test call does a yarn lint && yarn build before running tests which fixes such issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

core-logger-winston's logger.ts is unaffected because it just barely meets the length requirement.

@faustbrian faustbrian merged commit 8dffbb7 into ArkEcosystem:develop Dec 20, 2018
@ghost ghost removed the review label Dec 20, 2018
@paroxysm paroxysm deleted the feat/export-core-logger-winston-types branch December 20, 2018 22:15
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.

2 participants