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

Improve options typing #7

Merged
merged 1 commit into from
Jan 14, 2019
Merged

Conversation

pepakriz
Copy link
Contributor

No description provided.

@pepakriz pepakriz changed the title Dummy logger as default logger WIP: Dummy logger as default logger Jan 11, 2019
@pepakriz pepakriz changed the title WIP: Dummy logger as default logger WIP: Better options typing Jan 11, 2019
@coveralls
Copy link

coveralls commented Jan 11, 2019

Pull Request Test Coverage Report for Build 35

  • 7 of 7 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 98.28%

Totals Coverage Status
Change from base Build 32: -0.01%
Covered Lines: 824
Relevant Lines: 826

💛 - Coveralls

@pepakriz pepakriz changed the title WIP: Better options typing Better options typing Jan 11, 2019

const project = new Project();
const sources: SourceFile[] = [];

if (!this.em.options.entitiesDirsTs) {
Copy link
Member

@B4nan B4nan Jan 11, 2019

Choose a reason for hiding this comment

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

what is wrong with this? it is a problem when you enable more strict TS settings? i like to use this falsy comparison as its imho more readable and it is usually more accurate than strict check against undefined (what if you provide null, empty string, zero... still not valid but passes the condition).

lib/EntityFactory.ts Outdated Show resolved Hide resolved
lib/MikroORM.ts Outdated Show resolved Hide resolved
lib/MikroORM.ts Outdated Show resolved Hide resolved
@pepakriz
Copy link
Contributor Author

@B4nan it's ready to merge. A lot of thinks was removed, so PR is only about better Options typing. Next time I'll try to make smaller and more indipendent PRs.

BTW because the code coverage has been decreased (-0.01% 😄), PR is marked as failing. Is it necessary?

@B4nan B4nan changed the title Better options typing Improve options typing Jan 14, 2019
@B4nan B4nan merged commit 0df1114 into mikro-orm:master Jan 14, 2019
@B4nan
Copy link
Member

B4nan commented Jan 14, 2019

I guess that is just because you removed some tested code, don't worry about that :]

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.

3 participants