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

add TypeScript types #52

Merged
merged 1 commit into from
Oct 3, 2020
Merged

add TypeScript types #52

merged 1 commit into from
Oct 3, 2020

Conversation

bengry
Copy link

@bengry bengry commented Oct 3, 2020

This PR aims to add TypeScript types for the two entry points. Very helpful given that Cypress has built-in TypeScript support for quite some time, and helps with configuring the plugin.

The docs for the properties are based on the README ones.

* Output logs to files. [More details](https://github.com/archfz/cypress-terminal-report#logging-to-files).
* @default null
*/
outputTarget?: object | null;
Copy link
Owner

@archfz archfz Oct 3, 2020

Choose a reason for hiding this comment

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

Let's be more specific here. So Record<string, 'json' | 'txt' | (messages: Record<string, Record<string, [string, string, Severity]>) => string

Copy link
Author

Choose a reason for hiding this comment

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

Please see revised revision - I didn't see the callback API mentioned in the readme, but maybe I just missed it. LMK if this is what you meant.

@@ -1,14 +1,265 @@
{
Copy link
Author

Choose a reason for hiding this comment

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

package-lock.json diff was generated automatically when adding cypress as a dev dependency.

@@ -41,5 +41,8 @@
},
"peerDependencies": {
"cypress": ">=3.8.1"
},
"devDependencies": {
"cypress": ">=3.8.1"
Copy link
Author

Choose a reason for hiding this comment

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

needed to be able to reference the built-in Cypress types (in src/installLogsPrinter.d.ts).

@archfz archfz merged commit ae8b91c into archfz:master Oct 3, 2020
@archfz
Copy link
Owner

archfz commented Oct 3, 2020

Looks good :) Thx.

@archfz archfz mentioned this pull request Oct 3, 2020
@bengry bengry deleted the add-typescript-types branch October 3, 2020 15:18
@bengry
Copy link
Author

bengry commented Oct 3, 2020

Looks good :) Thx.

Thanks for going over it very quickly and merging it. I'll remove the relevant changes from our codebase once a new version is published.

Also, I'd appreciate it if you could add the hacktoberfest topic to this repo so this PR will be eligible for it. Due to some recent spamming, Hacktoberfest is now opt-in for repositories.

@archfz
Copy link
Owner

archfz commented Oct 3, 2020

Should I also add to this pr the hacktoberfest-accepted?

@bengry
Copy link
Author

bengry commented Oct 3, 2020

Should I also add to this pr the hacktoberfest-accepted?

I don't think it's required, on the other hand - it can't hurt. According to their explanation (highlights are mine):

PRs count if:
Submitted in a repo with the hacktoberfest topic AND
during the month of October AND (
The PR is merged OR
The PR is labelled as hacktoberfest-accepted by a maintainer OR
The PR has been approved
)

@archfz
Copy link
Owner

archfz commented Oct 3, 2020

Added just to be sure.

@bengry
Copy link
Author

bengry commented Oct 3, 2020

Added just to be sure.

Thanks! There's a cooldown of 2 weeks before anything is eligible anyway it seems, but I don't expect any issues coming up. Will let you know if there are, at any rate.

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.

2 participants