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

Feature/package test ts #584

Merged
merged 1 commit into from
Sep 22, 2018
Merged

Feature/package test ts #584

merged 1 commit into from
Sep 22, 2018

Conversation

ematipico
Copy link
Contributor

What kind of change does this PR introduce?
First typescript test

Did you add tests for your changes?
Yes

If relevant, did you update the documentation?
No

Summary
Added first typescript test, really small

Does this PR introduce a breaking change?
No

Other information

.prettierrc Outdated
@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

we can supply this cli wise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

Cli flag for this

@@ -0,0 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`info should return the information of the enviroment 1`] = `
Copy link
Member

Choose a reason for hiding this comment

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

use regular js for this, no need for snapshot against pure js objects

npmGlobalPackages: ["webpack", "webpack-cli"],
npmPackages: "*webpack*",
}),
await envinfo.run(information()),
Copy link
Member

Choose a reason for hiding this comment

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

does envinfo write to stdout by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know

Copy link
Member

Choose a reason for hiding this comment

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

Test it locally!

Copy link
Contributor Author

@ematipico ematipico Sep 13, 2018

Choose a reason for hiding this comment

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

Automatically, it doesn't, otherwise the stdout wouldn't be there

"build": "tsc",
"watch": "npm run build && tsc -w"
}
"name": "@webpack-cli/info",
Copy link
Member

Choose a reason for hiding this comment

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

is this from prettier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to figure this out. In theory it should follow .editorconfig

Copy link
Member

Choose a reason for hiding this comment

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

Ok, discard it

@webpack-bot
Copy link

@ematipico Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@ev1stensberg Please review the new changes.

tsconfig.json Outdated
"packages/*/node_modules/**"
]
"include": ["packages/**/*.ts"],
"exclude": ["node_modules/**", "packages/*/node_modules/**", "packages/*/__tests__/**"]
Copy link
Contributor Author

@ematipico ematipico Sep 13, 2018

Choose a reason for hiding this comment

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

We don't want to build the test files, so for now I added this match to exclude them

Copy link
Member

Choose a reason for hiding this comment

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

Okay!

@ematipico
Copy link
Contributor Author

Looks like commitlint is complaining due to the rebase. Can we skip that for now? We can squash with a fair commit

@evenstensberg
Copy link
Member

If the PR is approved we can do a squash

package.json Outdated
@@ -120,8 +114,6 @@
"codecov": "^3.1.0",
"commitizen": "^2.10.1",
"conventional-changelog-cli": "^2.0.5",
"conventional-changelog-lint-config-cz": "^0.3.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package has been archived

Copy link
Member

Choose a reason for hiding this comment

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

Why was it archived, any idea? I can't find anything on their readme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It now can be done without that package

package.json Outdated
@@ -86,14 +87,7 @@
"maxSize": "5.32 kB"
}
],
"config": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to archived packaged, I had to remove this configuration

@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@ematipico
Copy link
Contributor Author

Commit lint finally fixed :)

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

!!!😎👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants